Bug 225789 - Merge Review: genromfs
Merge Review: genromfs
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Petr Machata
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:42 EST by Nobody's working on this, feel free to take it
Modified: 2015-05-04 21:32 EDT (History)
3 users (show)

See Also:
Fixed In Version: 0.5.2-5.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-25 14:06:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
pmachata: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:42:33 EST
Fedora Merge Review: genromfs

http://cvs.fedora.redhat.com/viewcvs/devel/genromfs/
Initial Owner: pmachata@redhat.com
Comment 1 Petr Machata 2007-02-07 12:48:19 EST
Tidied up version commited, not built.
rpmlint is silent, for both source and binary rpm.
Comment 2 Kamil Dudka 2009-11-25 10:12:31 EST
Appended %{?dist} to the Release tag and built as genromfs-0.5.2-4.fc13.
Comment 3 Petr Machata 2009-11-25 10:51:34 EST
Clean fedora-review flag that the last reviewer left around.
Comment 4 Petr Machata 2009-11-25 10:52:01 EST
Starting formal review.
Comment 5 Petr Machata 2009-11-25 12:08:44 EST
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 12:40:42 EST
(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 13:41:32 EST
> > * 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 13:53:41 EST
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 14:06:36 EST
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.