Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 166317
Summary: | Review Request: perl-X11-Protocol | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Duncan Ferguson <duncan_j_ferguson> | ||||
Component: | Package Review | Assignee: | Paul Howarth <paul> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | tremble | ||||
Target Milestone: | --- | Keywords: | Reopened | ||||
Target Release: | --- | Flags: | kevin:
fedora-cvs+
|
||||
Hardware: | All | ||||||
OS: | Linux | ||||||
URL: | http://search.cpan.org/~smccam/X11-Protocol-0.54/Protocol.pm | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-08-29 05:40:32 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779 | ||||||
Attachments: |
|
Description
Duncan Ferguson
2005-08-19 02:40:02 UTC
Review: - package and spec naming OK - package meets guidelines - license is same as perl (with one or two exceptions noted in README, which are more liberal) - spec file written in English and is legible - sources match upstream - package builds fine on FC4 (i386) - no BR's needed - no locales, libraries, subpackages, pkgconfigs etc. to worry about - not relocatable - no directory ownership issues, but see below on clarity - no duplicate files - no permissions issues - %clean section present and correct - macro usage consistent - code, not content - no large docs - docs don't affect runtime - no scriptlets Needswork: - the test suite requires a running X server, so builds in mock will fail; what I would do is to edit the test script in %prep: # Testing requires X - use "rpmbuild --with X" %{!?_with_X:\ %{__perl} -pi -e 'print "print \"Remaining tests require X\n\"; exit 0;" if /Insert your test code below/;' test.pl \ } This will run the full test suite if the "--with X" option is passed to rpmbuild, else just the quick test that the module can be loaded. Nitpick: - rpmlint doesn't like shebangs (#!/usr/bin/perl) at the start of perl modules; not that they do any harm, but I'd suggest editing them out, e.g.in %prep: # Remove shebangs from module code find . -name '*.pm' -exec sed -i -e '/^#!\/usr\/bin\/perl$/d' {} ';' - BR: perl is redundant and should be deleted - license text not included in package; suggest adding to %prep: perldoc -t perlartistic > Artistic perldoc -t perlgpl > COPYING and to %files: %doc Artistic COPYING - setting of CFLAGS and OPTIMIZE is redundant for noarch packages - I'd include the examples directory as %doc - I'd change: %{perl_vendorlib}/X11* to: %{perl_vendorlib}/X11/ in the %files list as I think it's clearer that way I'm far from being a sed or perl guru so if you can think of better/neater ways of cleaning up these nitpicks, by all means use them. Sponsorship: Try reviewing a few of other people's packages - a list of packages awaiting review can be found at: https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-NEW&hide_resolved=1 That way, potential sponsors will gain confidence in your abilities as a packager. Created attachment 118159 [details]
Patch implementing review suggestions
Patch applied - I was expecting to have to do some work, but you did it all for me ;-) Updated and available at: Spec Url: http://queeg.dyndns.org/perl-X11-Protocol.spec SRPM Url: http://queeg.dyndns.org/perl-X11-Protocol-0.54-2.src.rpm OK, obviously I'm happy enough with the package as it is to approve it now. Do you have any plans for additional packages in Extras other than clusterssh? No plans at the moment - it seems hard enough finding time to do this while also juggling young kids and a wife ;-) You'll need to create a Fedora Account for yourself as described at http://fedoraproject.org/wiki/Extras/Contributors step 8. FWIW, my first young'un is due to arrive in the next week or so... Is it time for congratulations yet? Is there anything else I need to do before you (or someone else) approves the package? Due on Sunday. I'll sponsor you but you need to go through the accounts system first. I think i am already in there (under the user "duncs") but in a rejected state (as i applied for cvsextras too early in the process). Just tried adding myself again but got an error _pg.error: ERROR: duplicate key violates unique constraint "role_person_id_key" Are you able to change my status from "rejected" to "approved"? You should be OK now (did you get an email from the accounts system?). Looking at http://fedoraproject.org/wiki/Extras/Contributors it seems that the documentation is much improved on when I was going through this process :-) Attempted to resolve as "next release" but dont have the option to. Make sure you are in the "fedorabugs" group in the accounts system. https://admin.fedora.redhat.com/accounts/userbox.cgi Reopening bug to fix assignee. Asignee fixed, closing again. Package Change Request ====================== Package Name: perl-X11-Protocol New Branches: EL-5 Owners: tremble Have had a private email from Jima and he's happy for me to take on the EL branches cvs done. |