Bug 225789

Summary:	Merge Review: genromfs
Product:	[Fedora] Fedora	Reporter:	Nobody's working on this, feel free to take it <nobody>
Component:	Package Review	Assignee:	Petr Machata <pmachata>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Package Reviews List <fedora-package-review>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	kdudka, mnewsome, pmachata
Target Milestone:	---	Flags:	pmachata: fedora-review+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:	0.5.2-5.fc13	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2009-11-25 19:06:36 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

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!