[Bug 458402] Review Request: griffith - Media collection manager

[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=458402


Jon Ciesla <limb at jcomserv.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |limb at jcomserv.net




--- Comment #4 from Jon Ciesla <limb at jcomserv.net>  2008-10-15 10:02:54 EDT ---
Comments on practice review:
>[x] MUST: Each package must consistently use macros, as described in the macros
>   section of Packaging Guidelines.
>
>    Comment: missing %{name} macro in Source0: URL %{name} macro is used
>    consistently through out spec should be used here also.

This particular case is more of a convenience, not a necessity.  In fact,
sometimes I see macros overused, like %{version} in patch name, which must then
be hard-coded or renamed if the patch is valid over multiple versions, but this
is not the case here.

>One comment I forgot to mention. The Website URL appears to be a domain parker
>with pop-ups included. Is this correct or is this a temporary place holder for
>the website listed?

README gives http://www.griffith.cc, which is much better.

>[*] MUST: Packages containing GUI applications must include a %{name}.desktop
>    file, and that file must be properly installed with desktop-file-install
>    in the %install section. This is described in detail in the desktop files
>    section of the Packaging Guidelines . If you feel that your packaged GUI
>    application does not need a .desktop file, you must put a comment in the
>    spec file with your explanation.

Actually, this is not *quite* correctly handled:
https://fedoraproject.org/wiki/Packaging/Guidelines#desktop

Otherwise, a good review.

-- 
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.