Bug 503297 - Review Request: panoglview - Immersive viewer for spherical panoramas
Summary: Review Request: panoglview - Immersive viewer for spherical panoramas
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nicolas Chauvet (kwizart)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-30 22:47 UTC by Bruno Postle
Modified: 2009-07-27 11:32 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-07-27 11:32:34 UTC
Type: ---
Embargoed:
kwizart: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Bruno Postle 2009-05-30 22:47:40 UTC
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.

Comment 1 Fabian Affolter 2009-06-16 09:51:20 UTC
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

Comment 2 Bruno Postle 2009-06-17 19:23:59 UTC
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

Comment 3 Nicolas Chauvet (kwizart) 2009-07-01 15:18:21 UTC
- 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 ?)

Comment 4 Nicolas Chauvet (kwizart) 2009-07-01 16:13:31 UTC
There isn't any icon from the package and .desktop file !
Is it possible to pick one ?

Comment 5 Nicolas Chauvet (kwizart) 2009-07-01 16:18:19 UTC
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

Comment 6 Bruno Postle 2009-07-01 21:48:15 UTC
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.

Comment 7 Jason Tibbitts 2009-07-01 22:00:42 UTC
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.

Comment 8 Bruno Postle 2009-07-01 22:28:02 UTC
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.

Comment 9 Nicolas Chauvet (kwizart) 2009-07-06 23:13:15 UTC
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

-----------------------

Comment 10 Bruno Postle 2009-07-07 15:43:17 UTC
New Package CVS Request
=======================
Package Name: panoglview
Short Description: Immersive viewer for spherical panoramas
Owners: bpostle
Branches: F-11

Comment 11 Jason Tibbitts 2009-07-08 16:30:27 UTC
CVS done.

Comment 12 Fabian Affolter 2009-07-27 11:32:34 UTC
Package was built: http://koji.fedoraproject.org/koji/buildinfo?buildID=120355


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