Spec URL: http://bpostle.fedorapeople.org/reviews/panoglview/panoglview.spec SRPM URL: http://bpostle.fedorapeople.org/reviews/panoglview/panoglview-0.2.2-3.fc10.src.rpm Description: Use panoglview to explore equirectangular panoramic images. Equirectangular panoramas are typically JPEG/TIFF/PNG images with a 2:1 aspect ratio.
Just some quick comments on your spec file - You can remove '--vendor=""' - '%defattr(-, root, root)' should be '%defattr(-,root,root,-)' - Please add '%{?_smp_mflags}' to make
Thanks I've fixed %defattr and added %{?_smp_mflags}, I've left --vendor="" as this is required for it to build on el4 and el5: http://bpostle.fedorapeople.org/reviews/panoglview/panoglview.spec http://bpostle.fedorapeople.org/reviews/panoglview/panoglview-0.2.2-4.fc10.src.rpm I forgot to add that the way to test this package is to download some typical equirectangular panoramas and try to view them: http://www.flickr.com/search/?q=equirectangular&l=commderiv&ss=2&ct=6&w=all&adv=1
- starting review - OK - rpmlint panoglview is quiet on installed package OK - build in mock (fedora 11 x86_64 ) NEEDWORK - You aren't expected to run: update-mime-database %{_datadir}/mime Because this package doesn't bring any new mime type. But indeed update-desktop-database is mandatory since the .desktop file use a MimeType= field. As said: https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database By the way, using || : at the end of each scriptlet filters the error code if anything went wrong. So this only makes sense for the last line: example : first command &> /dev/null second command &> /dev/null || : That way, the rpm transaction will continue, even if update-desktop-database failed. NEEDWORK - License match source code : GPLv2+ but COPYING text is about GPLv3. This would need clarification NEEDWORK - url field cannot permit to download the source tarball (is it the right url ?) USABILITY test: I'm experiencing some refresh delay when moving the sphere. I will try to reproduce on another workstation. (Is it expected to see only from the inside despite of the outside of the sphere ?)
There isn't any icon from the package and .desktop file ! Is it possible to pick one ?
OKay, package works with radeon (x600se hardware) i686 F-11 whereas gives problem with nvidia lastest 3D driver F-11 x86_64. I will consider usability test to have succeeded
Thanks, updated spec and src.rpm here: http://bpostle.fedorapeople.org/reviews/panoglview/panoglview.spec http://bpostle.fedorapeople.org/reviews/panoglview/panoglview-0.2.2-5.fc11.src.rpm * removed the update-mime-database line. * changed License to GPLv3+. The COPYING file isn't in SVN and was added by automake at tarball build time. So the project is still GPLv2+, but the released tarball can only be interpreted as GPLv3+. I guess we are allowed to use 'any later version'. * fixed URL paste error in Source line. * I've added an icon based on the hugin icon.
Please note that the version of the GPL in the COPYING file has absolutely no bearing at all on the version of the GPL the software is under.
Regarding the license, what do you suggest? I see three options: * Delete COPYING and regenerate it with `automake --missing --copy` (result is a GPLv2 file). * Since we are entitled to upgrade to GPLv3 any time we like, just do that in the spec file. * Since the COPYING file has absolutely no bearing, ship it anyway and put GPLv2+ in the spec file.
Given the license of the build dependency,(wxWidgets, zlib, libtiff) which all are GPLv3 compatible we will not have a problem to move to GPLv3+ (like if any dependency were GPLv2 only). Now nothing requires us to be GPLv3+ either (exept if we consider COPYING text matter much than source code). Quoting the guidelines, we have to: - Have a license field - If a COPYING is provided, have it bundled. - Consider the binary rpm as the license field. What we don't have to do: - Have license field to match COPYING text. So this ends to be the #3rd choice which is the one of the src.rpm All other fields from the review have been corrected. ----------------------- This package (panoglview) is APPROVED by me -----------------------
New Package CVS Request ======================= Package Name: panoglview Short Description: Immersive viewer for spherical panoramas Owners: bpostle Branches: F-11
CVS done.
Package was built: http://koji.fedoraproject.org/koji/buildinfo?buildID=120355