[Bug 498218] Review Request: picturetile - Tiles a bunch of images into one large "photo wall"

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=498218


Orcan 'oget' Ogetbil <oget.fedora at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |oget.fedora at gmail.com
         AssignedTo|nobody at fedoraproject.org    |oget.fedora at gmail.com
               Flag|                            |fedora-review?




--- Comment #6 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-11-07 02:54:34 EDT ---
Thanks for packaging this nice small handy script. I made the full review on
this. The *'s are the only blockers. The rest is remarks and questions:

- rpmlint is silent

* The license is MIT
   https://fedoraproject.org/wiki/Licensing/MIT
old style. One of the oldest software licenses out there.

! Guidelines say: "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 must be included in %doc." in
   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text"
But this package does not even have a tarball. Is there any other examples of
Fedora packages that just cut the license text from the soure file? If not, I
would say it is best to obey the guideline.

? Shall we rename the executable to just picturetile? That is the upstream
project name.

! It would be better to install the executable with "install -pm 755 ..." and
not use %attr(0755,root,root) to comply with general Fedora conventions..

? I am a bit reluctant about the provides:
   Provides:       picturetile.pl = 20050314
Do we really need this?

? I would go with the suggestion in comment #2 for version and release numbers.
Even if upstream decides to make a 0.1 release we'll be in trouble.

* The first 5 lines of the %build should probably go into %prep. Also could you
use the "%setup -qcT" macro in %prep so that the package gets built in its own
directory?

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.