Bug 241262 - Review Request: libpciaccess - abstraction layer for PCI access
Summary: Review Request: libpciaccess - abstraction layer for PCI access
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Julian Sikorski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-05-24 17:40 UTC by Adam Jackson
Modified: 2007-11-30 22:12 UTC (History)
0 users

Fixed In Version: 0.8-0.1.20070530git.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-14 21:13:01 UTC
Type: ---
Embargoed:
belegdol: fedora-review+
tcallawa: fedora-cvs+


Attachments (Terms of Use)

Description Adam Jackson 2007-05-24 17:40:44 UTC
Spec URL: http://people.redhat.com/ajackson/libpciaccess/libpciaccess.spec
SRPM URL: http://people.redhat.com/ajackson/libpciaccess/libpciaccess-0.8-0.20070524.src.rpm
Description:
libpciaccess is an OS-independent abstraction layer for accessing PCI devices.  The X server will eventually depend on this, as well as several other projects if I have my way.

No upstream release tarball anywhere yet, hence the git snapshot in the SRPM.

Comment 1 Julian Sikorski 2007-05-30 10:36:03 UTC
      - MUST: rpmlint is not silent on the devel package. Complains about no
documentation, but this can be ignored.
      - MUST: The spec file name matches the base package name
      - MUST: The package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license and
meets other legal requirements as defined in the legal section of Packaging
Guidelines.
      - MUST: The License field in the package spec file must matches the actual
license - MIT
      - MUST: License is included in %doc.
      - MUST: The spec file is written in American English.
      - MUST: The spec file for the package is legible.
      - MUST: Package builds and compiles on fc6/i386
      - MUST: Did not find compile failures
      - MUST: Buildrequires are fine
      - MUST: Locale does not apply
      - MUST: ldconfig is used properly
      - MUST: Non-relocatable
      - MUST: No dirs created
      - MUST: No duplicates in %files
      - MUST: Correct permissions
      - MUST: %clean section present
      - MUST: Consistent use of macros
      - MUST: Package contains code
      - MUST: No large documentation
      - MUST: No %doc runtime dependency
      - MUST: Header files are in a -devel package.
      - MUST: No static libs
      - MUST: pkgconfig requires correct
      - MUST: .so file in -devel
      - MUST: Correct NVR requires for devel
      - MUST: No libtool archives
      - MUST: Not a GUI app
      - MUST: No overlapping ownership
      - MUST: Correct buildroot cleanup
      - MUST: All filenames in rpm packages are valid UTF-8.

SHOULD Items:

      - SHOULD: License is present as a separate file
      - SHOULD: Package builds in mock (fc6/i386)
      - SHOULD: Scriptlets are sane
      - SHOULD: NVR dependencies correct
      - SHOULD: pkgconfig files are in -devel

Things to correct:
- please change package release to 0.1.%{gitdate}git%{?dist}, as per [1]
- please read [2] to check how to state the source url precisely
- a minot thing: please align -devel requires with the lines above them

Other than that, the package looks fine to me.

[1]
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-cfd71146dbb6f00cec9fe3623ea619f843394837
[2]
http://fedoraproject.org/wiki/Packaging/SourceURL?highlight=%28sourceurl%29#head-615f6271efb394ab340a93a6cf030f2d08cf0d49

Comment 2 Julian Sikorski 2007-05-30 13:18:28 UTC
Ah, one more thing. Please add AUTHORS, ChangeLog, NEWS and README to %doc.

Comment 3 Adam Jackson 2007-05-31 03:56:31 UTC
(In reply to comment #2)
> Ah, one more thing. Please add AUTHORS, ChangeLog, NEWS and README to %doc.

NEWS and README are empty.  That seems unnecessary.  I'll happily add them once
they have content though.

New URLs:
http://people.redhat.com/ajackson/libpciaccess/libpciaccess.spec
http://people.redhat.com/ajackson/libpciaccess/libpciaccess-0.8-0.1.20070530.src.rpm

I added the git checkout instructions as a script since they're ugly to emit
inline.  The only tricky part is the bit where it removes .git/ before
compressing, since that's just wasteful to include in srpm.

Comment 4 Julian Sikorski 2007-05-31 08:45:33 UTC
I am getting 404 error on the srpm. Otherwise, spec looks fine to me, just don't
forget to update the %changelog section. It is not a blocker, so

APPROVED

Comment 5 Adam Jackson 2007-06-05 13:47:56 UTC
New Package CVS Request
=======================
Package Name: libpciaccess
Short Description: PCI access library
Owners: ajackson
Branches: F-7
InitialCC: 

Comment 6 Tom "spot" Callaway 2007-06-05 15:14:56 UTC
cvs done

Comment 7 Fedora Update System 2007-06-14 21:12:58 UTC
libpciaccess-0.8-0.1.20070530git.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


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