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 - Review Request: perl-X11-Protocol
Summary: Review Request: perl-X11-Protocol
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: David Lawrence
URL: http://search.cpan.org/~smccam/X11-Pr...
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-08-19 02:40 UTC by Duncan Ferguson
Modified: 2010-04-11 19:08 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-29 05:40:32 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)
Patch implementing review suggestions (1.79 KB, patch)
2005-08-26 15:33 UTC, Paul Howarth
no flags Details | Diff

Description Duncan Ferguson 2005-08-19 02:40:02 UTC
Please note, this is my first package and I need a sponsor.

Spec Url: http://queeg.dyndns.org/perl-X11-Protocol.spec
SRPM Url: http://queeg.dyndns.org/perl-X11-Protocol-0.54-1.src.rpm
Description: 

X11::Protocol is a client-side interface to the X11 Protocol (see X(1) for
information about X11), allowing perl programs to display windows and
graphics on X11 servers.

Comment 1 Paul Howarth 2005-08-26 15:31:54 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.




Comment 2 Paul Howarth 2005-08-26 15:33:10 UTC
Created attachment 118159 [details]
Patch implementing review suggestions

Comment 3 Duncan Ferguson 2005-08-30 18:15:53 UTC
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

Comment 4 Paul Howarth 2005-08-31 08:47:09 UTC
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?


Comment 5 Duncan Ferguson 2005-08-31 18:39:09 UTC
No plans at the moment - it seems hard enough finding time to do this while also
juggling young kids and a wife ;-)

Comment 6 Paul Howarth 2005-09-01 15:29:48 UTC
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...


Comment 7 Duncan Ferguson 2005-09-07 17:13:58 UTC
Is it time for congratulations yet?

Is there anything else I need to do before you (or someone else) approves the
package?

Comment 8 Paul Howarth 2005-09-07 17:29:46 UTC
Due on Sunday.

I'll sponsor you but you need to go through the accounts system first.

Comment 9 Duncan Ferguson 2005-09-07 19:24:04 UTC
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"?

Comment 10 Paul Howarth 2005-09-08 08:53:11 UTC
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 :-)


Comment 11 Duncan Ferguson 2005-10-14 08:24:37 UTC
Attempted to resolve as "next release" but dont have the option to.

Comment 12 Paul Howarth 2005-10-14 08:55:35 UTC
Make sure you are in the "fedorabugs" group in the accounts system.

https://admin.fedora.redhat.com/accounts/userbox.cgi


Comment 13 Paul Howarth 2006-08-29 05:38:18 UTC
Reopening bug to fix assignee.

Comment 14 Paul Howarth 2006-08-29 05:40:32 UTC
Asignee fixed, closing again.

Comment 15 Mark Chappell 2010-04-09 07:58:43 UTC
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

Comment 16 Kevin Fenzi 2010-04-11 19:08:42 UTC
cvs done.


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