[Bug 710452] Review Request: unzix - A WinZix archive extractor

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

--- Comment #3 from Nils Philippsen <nphilipp at redhat.com> 2011-06-03 17:10:29 EDT ---
Hi Mukund, thanks for submitting this package and starting as a Fedora
contributor!

Jussi, in the words of the great Bob Geldof: "I don't mind at all." :-)

Anyway, here's the review probably overlapping a bit with Jussi's comments:

Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this
package. Read them anyway as they may still contain valuable hints.
Items marked "CHECK" either aren't covered by the guidelines or unclear, but
you should check and fix them anyway in my opinion.
Items marked "BAD" violate the guidelines in some point and need to be fixed.

BAD: rpmlint indicates errors:

    unzix.x86_64: E: explicit-lib-dependency zlib
    unzix.x86_64: W: empty-%pre
    unzix.x86_64: W: empty-%post
    unzix.x86_64: W: empty-%preun
    unzix.x86_64: W: empty-%postun
    3 packages and 1 specfiles checked; 1 errors, 4 warnings.

    --> remove "Requires: zlib", empty scriptlets

GOOD: package named according to guidelines
GOOD: spec file named like package
BAD: The license as in LICENSE is acceptable for Fedora, but unzix.c is
licensed as "Copyright (C) 2009 Mukund Sivaraman. All rights reserved." without
a mention of the BSD license. I know you're a good guy and won't use this
against someone, but this needs to be fixed ;-). For the remainder of the
review I'm assuming that the stated new BSD license holds.
GOOD: spec file license matches package license
GOOD: license text file included as %doc
CHECK: Spec file must be written in American English: In the description it's
probably rather "... extracting files from archives in the WinZix format." (I'd
leave out "new" as this will change over time) and the changelog entry should
rather be "Initial rpm package" or "... packaging" IMO. Language lawyering
works best late in the evening ;-).
GOOD: spec file is legible
GOOD: The sources used to build the package match the tarball upstream. NB,
just for completeness sake and not because I think you would do that ;-): As
the upstream maintainer of this package, please don't "recycle" tarball
versions, if the tarball has new contents, the version should be bumped.
GOOD: the package successfully compiles and builds into a package on
F-15/x86_64
GOOD: all build requirements listed in spec file
PASS: no locale specific files (yet ;-), read
http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files when
it comes to that)
PASS: no shared libraries included
GOOD: no copies of system libraries included
PASS: package not relocatable
PASS: package doesn't explicitly create directories
GOOD: doesn't list packages files more than once
GOOD: File permissions are set properly. Note that you technically don't need
the %defattr statement in the %files section for Fedora (i.e. rpm > 4.4), but
it probably is convenient for people doing rebuilds on older systems.
GOOD: consistently uses macros
GOOD: package contains code and permissible content
PASS: no large documentation files
GOOD: package doesn't depend on %doc files during runtime
PASS: no header files packaged
PASS: no static libraries packaged
PASS: no libraries, with or without version suffix
PASS: no devel package
PASS: no libtool archives (*.la) created
PASS: doesn't contain GUI applications
GOOD: doesn't own files or directories owned by other packages
GOOD: all file names are valid UTF-8
GOOD: contains man page for the included unzix binary

Please fix the (few) found issues, I'm confident we can get this done in the
next round.

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