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 225789 - Merge Review: genromfs
Summary: Merge Review: genromfs
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Machata
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 18:42 UTC by Nobody's working on this, feel free to take it
Modified: 2015-05-05 01:32 UTC (History)
3 users (show)

Fixed In Version: 0.5.2-5.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-25 19:06:36 UTC
pmachata: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 18:42:33 UTC
Fedora Merge Review: genromfs

http://cvs.fedora.redhat.com/viewcvs/devel/genromfs/
Initial Owner: pmachata@redhat.com

Comment 1 Petr Machata 2007-02-07 17:48:19 UTC
Tidied up version commited, not built.
rpmlint is silent, for both source and binary rpm.

Comment 2 Kamil Dudka 2009-11-25 15:12:31 UTC
Appended %{?dist} to the Release tag and built as genromfs-0.5.2-4.fc13.

Comment 3 Petr Machata 2009-11-25 15:51:34 UTC
Clean fedora-review flag that the last reviewer left around.

Comment 4 Petr Machata 2009-11-25 15:52:01 UTC
Starting formal review.

Comment 5 Petr Machata 2009-11-25 17:08:44 UTC
The package is OK overall, I found just a couple nits:

* rpmlint report:
    $ rpmlint x86_64/genromfs-0.5.2-4.fc13.x86_64.rpm \
        genromfs-0.5.2-4.fc13.src.rpm genromfs.spec 
    2 packages and 1 specfiles checked; 0 errors, 0 warnings.

* ExclusiveOS: I don't see a reason to state this explicitly.  The
  fact that it's a Fedora package guarantees that it will only ever be
  compiled for Linux machines.

* BuildRoot tag is ignored in Fedora 10+.  Consider removing it.

* Package COPYING, and consider also packaging NEWS (via %doc)

* %files section should include %defattr(-,root,root,-) (Notice the
  last dash.)

Comment 6 Kamil Dudka 2009-11-25 17:40:42 UTC
(In reply to comment #5)

Thanks for review! I've just committed the proposed changes to CVS, excluding the BuildRoot related one. (+ fixed spelling error with the latest rpmlint filesystem --> file system)

> * ExclusiveOS: I don't see a reason to state this explicitly.  The
>   fact that it's a Fedora package guarantees that it will only ever be
>   compiled for Linux machines.

Good idea. 'cvs annotate' says the line has been there since the initial import.

> * BuildRoot tag is ignored in Fedora 10+.  Consider removing it.

However removing it triggers a warning with the latest rpmlint:

$ rpmlint --version
rpmlint version 0.92 Copyright (C) 1999-2007 Frederic Lepied, Mandriva
$ rpmlint genromfs.spec
genromfs.spec: W: no-buildroot-tag
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

> * Package COPYING, and consider also packaging NEWS (via %doc)

Fixed.

> * %files section should include %defattr(-,root,root,-) (Notice the
>   last dash.)  

Fixed.

Comment 7 Petr Machata 2009-11-25 18:41:32 UTC
> > * BuildRoot tag is ignored in Fedora 10+.  Consider removing it.
> 
> However removing it triggers a warning with the latest rpmlint:

Hmm, right.  I think we could overrule rpmlint at this point, it's complaining about something that's in fact not mandatory per the guidelines.  But I'll leave that up to you, I don't have any strong feelings either way.

I think the package is now OK and I'm going to ACCEPT it.  Thanks for cooperation.

Comment 8 Petr Machata 2009-11-25 18:53:41 UTC
The rest of the process doesn't really apply to merge review, so just close the review request as soon as you build that package with those fixes in.

Comment 9 Kamil Dudka 2009-11-25 19:06:36 UTC
Built as genromfs-0.5.2-5.fc13. Thanks!


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