Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226447 - Merge Review: sysfsutils
Summary: Merge Review: sysfsutils
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Todd Zullinger
QA Contact: Fedora Package Reviews List
Depends On:
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
Reported: 2007-01-31 21:03 UTC by Nobody's working on this, feel free to take it
Modified: 2008-08-02 23:40 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2008-02-25 22:25:58 UTC
tmz: fedora-review+

Attachments (Terms of Use)
patch to fix minor issues for merge review (deleted)
2008-01-11 19:16 UTC, Todd Zullinger
no flags Details | Diff

Description Nobody's working on this, feel free to take it 2007-01-31 21:03:50 UTC
Fedora Merge Review: sysfsutils
Initial Owner:

Comment 2 Jarod Wilson 2007-02-04 17:50:15 UTC
Should actually be assigned to until someone picks it up for review, only needs 
to be assigned over to me once a reviewer says there's something that needs my attention.

Comment 3 Todd Zullinger 2008-01-11 19:15:16 UTC
Jarod, here's a review.  Things look mostly sane.  The few minor issues are
noted below.  I'll attach a patch that makes these changes.  Feel free to use
some or all of it, with or without attribution. ;)

* source files match upstream:

b3cb316c652b09ec66f93f4ea98a93a7a1001678  sysfsutils-2.1.0.tar.gz

* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
    %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

X license field matches the actual license.
    The main package license should be GPLv2
    (cmd/systool.c specifies v2 and has no "or any later version clause")

    The libsysfs subpackage should be LGPLv2+

* license is open source-compatible.
* latest version is being packaged.
* BuildRequires are proper (none are needed).
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (fedora-devel-x86_64,).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent. (not quite, but these warnings should be okay to ignore):

    sysfsutils.x86_64: W: incoherent-version-in-changelog 2.1.0-2 2.1.0-1.fc9
    libsysfs-devel.x86_64: W: no-documentation

* final provides and requires are sane
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.

X scriptlets are properly run for libs
    ldconfig needs to be run for the libsysfs subpackage

* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

Comment 4 Todd Zullinger 2008-01-11 19:16:29 UTC
Created attachment 291411 [details]
patch to fix minor issues for merge review

Comment 5 Jarod Wilson 2008-02-25 22:25:58 UTC
D'oh. This got lost in the shuffle... Just made these changes (more or less) to
rawhide, building now.

Comment 6 Kevin Fenzi 2008-02-26 23:53:57 UTC
Todd: Did you want to mark this with fedora-review + ? 

Jarod: You shouldn't usually close until it's been approved... ;) 

Comment 7 Todd Zullinger 2008-02-27 00:42:20 UTC
Yep, I wanted to look back over it before I added the fedora-review +, since it
was a month ago that I did the review and my memory needs frequent refreshing. 
It looks like everything is good.  Thanks Jarod (and Kevin for poking :).

Comment 8 Jarod Wilson 2008-02-27 04:00:33 UTC
Whoops, sorry, my mistake... Got close-happy plowing through bugs yesterday... :)

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