Bug 528010 - Review Request: PyMca - GUI for multi-channel analyser spectra visualization and analysis
Summary: Review Request: PyMca - GUI for multi-channel analyser spectra visualization ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Thomas Spura
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-08 15:23 UTC by Susi Lehtola
Modified: 2010-03-29 06:08 UTC (History)
4 users (show)

Fixed In Version: 4.3.0-4.fc12
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-01-26 00:56:39 UTC
Type: ---
Embargoed:
tomspur: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Susi Lehtola 2009-10-08 15:23:37 UTC
Spec URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca.spec

SRPM URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/PyMca-4.3.0-1.fc11.src.rpm

Upstream URL:
http://pymca.sourceforge.net/

Description:
PyMCA provides a graphical interface for multi-channel analyser spectra
visualization and analysis. PyMca can display spectra from a file or directly
from SPEC during acquisitions.

rpmlint output is clean.

Comment 1 Thomas Spura 2009-10-30 15:21:18 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?

Comment 2 Susi Lehtola 2009-10-30 15:34:46 UTC
(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

Comment 3 Thomas Spura 2009-10-30 17:00:12 UTC
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)

Comment 4 Susi Lehtola 2009-10-30 17:19:32 UTC
(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

Comment 5 Thomas Spura 2009-10-30 17:40:26 UTC
(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

Comment 6 Susi Lehtola 2009-10-30 19:18:43 UTC
(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:

Comment 7 Thomas Spura 2009-10-30 19:56:31 UTC
(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.

Comment 8 Kevin Fenzi 2009-10-31 23:50:33 UTC
cvs done.

Comment 9 Fedora Update System 2010-01-07 07:43:17 UTC
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

Comment 10 Fedora Update System 2010-01-07 21:44:28 UTC
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

Comment 11 Fedora Update System 2010-01-26 00:56:34 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.