Bug 241262 - Review Request: libpciaccess - abstraction layer for PCI access
Review Request: libpciaccess - abstraction layer for PCI access
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Julian Sikorski
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-24 13:40 EDT by Adam Jackson
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version: 0.8-0.1.20070530git.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-14 17:13:01 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
belegdol: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Adam Jackson 2007-05-24 13:40:44 EDT
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 06:36:03 EDT
      - 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 09:18:28 EDT
Ah, one more thing. Please add AUTHORS, ChangeLog, NEWS and README to %doc.
Comment 3 Adam Jackson 2007-05-30 23:56:31 EDT
(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 04:45:33 EDT
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 09:47:56 EDT
New Package CVS Request
=======================
Package Name: libpciaccess
Short Description: PCI access library
Owners: ajackson@redhat.com
Branches: F-7
InitialCC: 
Comment 6 Tom "spot" Callaway 2007-06-05 11:14:56 EDT
cvs done
Comment 7 Fedora Update System 2007-06-14 17:12:58 EDT
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.