Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: unzix - A WinZix archive extractor
https://bugzilla.redhat.com/show_bug.cgi?id=710452
Summary: Review Request: unzix - A WinZix archive extractor Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: muks@banu.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora Story Points: ---
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.
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
Nils Philippsen nphilipp@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |nphilipp@redhat.com Blocks| |177841(FE-NEEDSPONSOR) AssignedTo|nobody@fedoraproject.org |nphilipp@redhat.com Flag| |fedora-review?
--- Comment #1 from Nils Philippsen nphilipp@redhat.com 2011-06-03 10:29:15 EDT --- I'm taking this review. Added FE-NEEDSPONSOR to blockers.
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
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jussi.lehtola@iki.fi
--- Comment #2 from Jussi Lehtola jussi.lehtola@iki.fi 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.
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@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.
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 #4 from Mukund Sivaraman muks@banu.com 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. :)
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
Nils Philippsen nphilipp@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #5 from Nils Philippsen nphilipp@redhat.com 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.
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 #6 from Mukund Sivaraman muks@banu.com 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. :)
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
Mukund Sivaraman muks@banu.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #7 from Mukund Sivaraman muks@banu.com 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:
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 #8 from Jon Ciesla limb@jcomserv.net 2011-06-06 11:52:05 EDT --- Git done (by process-git-requests).
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
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
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 #9 from Fedora Update System updates@fedoraproject.org 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
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 #10 from Fedora Update System updates@fedoraproject.org 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
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 #11 from Fedora Update System updates@fedoraproject.org 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
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
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #12 from Fedora Update System updates@fedoraproject.org 2011-06-07 19:04:41 EDT --- unzix-0.3.0-1.el6 has been pushed to the Fedora EPEL 6 testing repository.
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 #13 from Fedora Update System updates@fedoraproject.org 2011-08-08 21:25:33 EDT --- unzix-0.3.0-1.fc15 has been pushed to the Fedora 15 stable repository.
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
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |unzix-0.3.0-1.fc15 Resolution| |ERRATA Last Closed| |2011-08-08 21:25:39
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 #14 from Fedora Update System updates@fedoraproject.org 2011-08-08 21:32:25 EDT --- unzix-0.3.0-1.fc14 has been pushed to the Fedora 14 stable repository.
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
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|unzix-0.3.0-1.fc15 |unzix-0.3.0-1.fc14
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 #15 from Fedora Update System updates@fedoraproject.org 2011-08-09 17:59:55 EDT --- unzix-0.3.0-1.el6 has been pushed to the Fedora EPEL 6 stable repository.
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
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|unzix-0.3.0-1.fc14 |unzix-0.3.0-1.el6
package-review@lists.fedoraproject.org