Bug 710452 - Review Request: unzix - A WinZix archive extractor
Review Request: unzix - A WinZix archive extractor
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nils Philippsen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-03 08:44 EDT by Mukund Sivaraman
Modified: 2013-10-19 10:42 EDT (History)
4 users (show)

See Also:
Fixed In Version: unzix-0.3.0-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-08-08 21:25:39 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
nphilipp: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mukund Sivaraman 2011-06-03 08:44:47 EDT
Spec URL: http://mukund.org/tmp/unzix.spec
SRPM URL: http://mukund.org/tmp/unzix-0.2.0-1.fc14.src.rpm
Description: Unzix is a small command-line program for extracting files from the new WinZix archive format.

I am a new packager and will need a sponsor. :)

Unzix is a program I have written myself (i.e., I'm the upstream). It is BSD licensed and uses zlib as a dependency.
Comment 1 Nils Philippsen 2011-06-03 10:29:15 EDT
I'm taking this review. Added FE-NEEDSPONSOR to blockers.
Comment 2 Susi Lehtola 2011-06-03 14:11:02 EDT
Yet another wrapper around gzip..?

A few notes, I hope Nils won't mind :)

- Drop the empty directives
%pre
%post
%preun
%postun

- Drop the explicit Requires: on zlib, it is picked up automatically by RPM.
  (It's also forbidden in the Fedora Packaging guidelines at
   http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires )

- In case the man page compression format changes, it's better to use
 %{_mandir}/man1/%{name}.1.*
instead of
 %{_mandir}/man1/%{name}.1.gz

Otherwise looks good to me.
Comment 3 Nils Philippsen 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.
Comment 4 Mukund Sivaraman 2011-06-04 02:39:44 EDT
Hi Nils, Jussi :)

Thank you for reviewing it so quickly. I have made the necessary changes:

(In reply to comment #3)
> 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.

I have changed the source code accordingly, and have made a new release. You can browse the code here: https://banu.com/cgit/unzix/

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

Removed.

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

Removed.

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

Changed.

(In reply to comment #2)
> - In case the man page compression format changes, it's better to use
>  %{_mandir}/man1/%{name}.1.*
> instead of
>  %{_mandir}/man1/%{name}.1.gz

Changed.

----

The new files can be browsed here:

http://mukund.org/tmp/unzix.spec-2
  (the "-2" is a temporary URL suffix so you can diff the changes from orig)
http://mukund.org/tmp/unzix-0.3.0-1.fc14.src.rpm

Please review them and tell me if all looks good. :)
Comment 5 Nils Philippsen 2011-06-06 05:50:01 EDT
BAD: rpmlint: changelog entry version (0.2.0-1) doesn't match package version (0.3.0-1):

    unzix.x86_64: W: incoherent-version-in-changelog 0.2.0-1 ['0.3.0-1.fc15', '0.3.0-1']
    3 packages and 1 specfiles checked; 0 errors, 1 warnings.

GOOD: license in files matches intended license and license in spec file
GOOD: description and changelog text amended
GOOD: catches all possible compression formats for man pages

Provided that you fix the changelog entry (version, date) before importing, this package is APPROVED. You may now proceed with the next steps in the review process -- https://fedoraproject.org/wiki/Package_Review_Process#Contributor -- to get your package imported, built and published. This process is finished when you (or Bodhi, the package update system) closes this bug.

Welcome as a new Fedora contributor! I've sponsored you for the packager group so you should have all necessary permissions to work on your package. I'll also keep an eye on Fedora bugs which are assigned to you. Feel free to ask me anything about processes etc. so you can smoothly contribute.
Comment 6 Mukund Sivaraman 2011-06-06 09:21:54 EDT
(In reply to comment #5)
> BAD: rpmlint: changelog entry version (0.2.0-1) doesn't match package version
> (0.3.0-1):
> 
>     unzix.x86_64: W: incoherent-version-in-changelog 0.2.0-1 ['0.3.0-1.fc15',
> '0.3.0-1']
>     3 packages and 1 specfiles checked; 0 errors, 1 warnings.

I've fixed this locally, and will upload with the fixed spec file.

> GOOD: license in files matches intended license and license in spec file
> GOOD: description and changelog text amended
> GOOD: catches all possible compression formats for man pages
> 
> Provided that you fix the changelog entry (version, date) before importing,
> this package is APPROVED. You may now proceed with the next steps in the review
> process -- https://fedoraproject.org/wiki/Package_Review_Process#Contributor --
> to get your package imported, built and published. This process is finished
> when you (or Bodhi, the package update system) closes this bug.
> 
> Welcome as a new Fedora contributor! I've sponsored you for the packager group
> so you should have all necessary permissions to work on your package. I'll also
> keep an eye on Fedora bugs which are assigned to you. Feel free to ask me
> anything about processes etc. so you can smoothly contribute.

Thank you! Now onward to the next steps. :)
Comment 7 Mukund Sivaraman 2011-06-06 09:25:21 EDT
New Package SCM Request
=======================
Package Name: unzix
Short Description: A WinZix archive extractor
Owners: muks
Branches: f14 f15 el4 el5 el6
InitialCC:
Comment 8 Jon Ciesla 2011-06-06 11:52:05 EDT
Git done (by process-git-requests).
Comment 9 Fedora Update System 2011-06-07 10:52:22 EDT
unzix-0.3.0-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/unzix-0.3.0-1.fc14
Comment 10 Fedora Update System 2011-06-07 10:52:31 EDT
unzix-0.3.0-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/unzix-0.3.0-1.el6
Comment 11 Fedora Update System 2011-06-07 10:52:39 EDT
unzix-0.3.0-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/unzix-0.3.0-1.fc15
Comment 12 Fedora Update System 2011-06-07 19:04:41 EDT
unzix-0.3.0-1.el6 has been pushed to the Fedora EPEL 6 testing repository.
Comment 13 Fedora Update System 2011-08-08 21:25:33 EDT
unzix-0.3.0-1.fc15 has been pushed to the Fedora 15 stable repository.
Comment 14 Fedora Update System 2011-08-08 21:32:25 EDT
unzix-0.3.0-1.fc14 has been pushed to the Fedora 14 stable repository.
Comment 15 Fedora Update System 2011-08-09 17:59:55 EDT
unzix-0.3.0-1.el6 has been pushed to the Fedora EPEL 6 stable repository.

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