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
Clone Of:
Environment:
Last Closed: 2009-11-25 19:06:36 UTC
Type: ---
Embargoed:
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

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.