Bug 528010
Summary: | Review Request: PyMca - GUI for multi-channel analyser spectra visualization and analysis | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Susi Lehtola <susi.lehtola> |
Component: | Package Review | Assignee: | Thomas Spura <tomspur> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mail, notting, tomspur |
Target Milestone: | --- | Flags: | tomspur:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 4.3.0-4.fc12 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2010-01-26 00:56:39 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Susi Lehtola
2009-10-08 15:23:37 UTC
Just a few comments for now: - Don't you want to name it PyMCA? - BR: python-setuptools-devel is not needed, python-setuptools is enought. - get rid of /usr/bin/env: find %{buildroot}%{python_sitearch}/PyMca -name '*.py' | xargs sed -i '/\/usr\/bin\/env/d' is much less noisy, than your for loop and 'seems' to act faster, but your choice ;). The for loop is just necessary, if you want to keep timestamps… Hmm, why don't you? - What do you want to archieve with 'echo yes' in %build and %install? (In reply to comment #1) > Just a few comments for now: > > - Don't you want to name it PyMCA? http://pymca.sourceforge.net/faq.html#mozTocId556491 "Due to some problems I encountered with the publisher, I have some preference for PyMca because it makes clear that it is the name of the program and it does not intend to be an acronym." > - BR: python-setuptools-devel is not needed, python-setuptools is enought. Done for the next release. > - get rid of /usr/bin/env: > find %{buildroot}%{python_sitearch}/PyMca -name '*.py' | xargs sed -i > '/\/usr\/bin\/env/d' > is much less noisy, than your for loop and 'seems' to act faster, but your > choice ;). The for loop is just necessary, if you want to keep timestamps… > Hmm, why don't you? Keeping time stamps. > - What do you want to archieve with 'echo yes' in %build and %install? The stupid setup macro asks for (GPL) license approval. http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca.spec http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca-4.3.0-2.fc12.src.rpm Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on: [] devel/i386 [] devel/x86_64 [] F11/i386 [x] F11/x86_64 [?] Rpmlint output: $ rpmlint PyMca.spec PyMca-4.3.0-2.fc11.src.rpm x86_64/PyMca-* 3 packages and 1 specfiles checked; 0 errors, 0 warnings. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x] License field in the package spec file matches the actual license. License type: GPLv2+ [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. Upstream source: e9d986a14899fecf38e5214c5712d409 Build source: e9d986a14899fecf38e5214c5712d409 [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot}. [x] Package consistently uses macros. [x] Package contains code, or permissable content. [x] Large documentation files are in a -doc subpackage, if required. Some data could get into a -data subpackage, but this is unneeded here. [x] Package uses nothing in %doc for runtime. Package uses changelog.txt for runtime, but this is in the main package, OK. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [!] Package contains a properly installed %{name}.desktop file if it is a GUI application. desktopfile is missing... [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [!] Package should compile and build into binary rpms on all supported architectures. Missing BR: python-devel (gcc fails because Python.h is not found) With that, it builds in koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1779190 Issues: - no desktop file (MUST) - keeping timestamps are not everywhere (no must) - BR: python-devel (MUST) (In reply to comment #3) > Issues: > - no desktop file (MUST) Added. > - keeping timestamps are not everywhere (no must) What are you referring to..? > - BR: python-devel (MUST) Yes, this is needed now since it isn't pulled in anymore by python-setuptools-devel. http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca.spec http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca-4.3.0-3.fc12.src.rpm (In reply to comment #4) > (In reply to comment #3) > > Issues: > > - no desktop file (MUST) > > Added. Ok, there is no icon present, you could install PyMca4.3.0/PyMca/PyMcaSplashImage.png into pixmaps and use that... > > > - keeping timestamps are not everywhere (no must) > What are you referring to..? e.g. # Get rid of buildroot in files in %{_bindir} for bin in %{buildroot}%{_bindir}/*; do sed -i 's|%{buildroot}||g' $bin done The same as the other loop, where you preserve them now. > > > - BR: python-devel (MUST) > > Yes, this is needed now since it isn't pulled in anymore by > python-setuptools-devel. Ok, on my system I have only installed python-devel and python-setuptools… -devel is not needed, that's why I complained about this ;) ____________________________ Only minor fixes can be done: installing an icon and the timestamps. || VV ____________________________ APPROVED (In reply to comment #5) > Ok, there is no icon present, you could install > PyMca4.3.0/PyMca/PyMcaSplashImage.png into pixmaps and use that... file reports PyMca/PyMcaSplashImage.png: PNG image, 400 x 300, 8-bit/color RGB, non-interlaced so I don't think it's OK to use it (files in /usr/share/pixmaps are a lot smaller, and have square format). ** Thanks for the review! New Package CVS Request ======================= Package Name: PyMca Short Description: GUI for multi-channel analyser spectra visualization and analysis Owners: jussilehtola Branches: F-11 F-12 InitialCC: (In reply to comment #6) > file reports > PyMca/PyMcaSplashImage.png: PNG image, 400 x 300, 8-bit/color RGB, > non-interlaced > > so I don't think it's OK to use it (files in /usr/share/pixmaps are a lot > smaller, and have square format). From file : 126 pixmap = qt.QPixmap.fromMimeSource('PyMcaSplashImage.png') 127 splash = qt.QSplashScreen(pixmap) 128 splash.show() It seems, they are using this image as image in the upper left corner. cvs done. PyMca-4.3.0-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/PyMca-4.3.0-4.fc12 PyMca-4.3.0-4.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update PyMca'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-0272 PyMca-4.3.0-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report. |