[Bug 467179] Review Request: pngcrush - Optimizer for PNG (Portable Network Graphics) files

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





--- Comment #6 from Jerry James <loganjerry at gmail.com>  2008-10-31 16:39:40 EDT ---
Sorry to be paranoid about the license, but reviewers are supposed to be
paranoid, aren't they? :-)  Speaking of paranoia, would you mind also removing
crc32.h, deflate.h, inf*.h, and trees.h before compiling?  It probably doesn't
matter, but it will give me warm fuzzies.  Also, it doesn't appear necessary to
link with -lm, since that gets pulled in by libpng.  And Patch0 is now
unnecessary since you aren't using make any more.  Did you consider making
Changelog.txt a %doc file?  Finally, according to
https://fedoraproject.org/wiki/Packaging/SourceURL the source URL should refer
to downloads.sourceforge.net.

MUST items:
- rpmlint output:
pngcrush.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
- package name: OK
- spec file name: OK
- Packaging Guidelines: OK, except for the source URL and Patch0
- approved license: OK
- license field: OK
- license file: OK
- spec in American English: OK
- legible spec: OK
- package sources match upstream: OK
- successful compile on one arch: OK
- ExcludeArch: OK
- BuildRequires: OK
- locales: OK
- library files: OK
- relocatable: OK
- own all directories: OK
- no duplicates files: OK
- file permissions: OK
- %clean section: OK
- consistent use of macros: OK
- code or permissible content: OK
- large documentation files: OK
- no runtime files in %doc: OK
- header files: OK
- static libraries: OK
- pkgconfig files: OK
- library files with no suffix in devel: OK
- devel package Requires: OK
- no libtool archives: OK
- desktop files: OK
- do not own files/dirs owned by other packages: OK
- clean at start of %install: OK
- filenames are UTF-8: OK

SHOULD items:
- query upstream for a license file: have you done this?
- description and summary translations: OK
- builds in mock: OK
- builds into packages on all supported arches: did not check
- package functions as described: OK with light testing
- sane scriptlets: OK
- subpackages require main package: OK
- pkgconfig files in devel: OK
- file dependencies: OK

I believe the source URL must be changed, because it is referred to from the
Packaging Guidelines, and so falls under a MUST item.

The unnecessary Patch0 also falls afoul of the Packaging Guidelines, I believe,
due to the section entitled "All patches should have an upstream bug link or
comment".  Just remove it.

Please also consider the following (but I won't block approval for these):
- Also remove crc32.h, deflate.h, inf*.h, and trees.h before compiling
- Drop "-lm" from the link line
- Make Changelog.txt a %doc file

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