Bug 210757

Summary:	Review Request: magicor - Push ice blocks around to extenguish all fires
Product:	[Fedora] Fedora	Reporter:	Hans de Goede <hdegoede>
Component:	Package Review	Assignee:	Paul F. Johnson <paul>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Package Reviews List <fedora-package-review>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	wart
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2006-10-21 20:48:37 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:
Bug Depends On:
Bug Blocks:	163779

Description Hans de Goede 2006-10-14 11:48:43 UTC

Spec URL: http://people.atrpms.net/~hdegoede/magicor.spec
SRPM URL: http://people.atrpms.net/~hdegoede/magicor-0.1-1.fc6.src.rpm
Description:
The goal of the game is to annihilate all burning fires. You do this
by pushing blocks of ice until they collide with a burning fire.
When the ice blocks hit burning fire the block and the fire are destroyed.
Once all fires are extinguished the level is completed.

Comment 1 Paul F. Johnson 2006-10-14 12:27:07 UTC

rpmlint warning/errors

.src.rpm - no %build section (W)
.noarch.rpm - non-conffile-in-etc /etc/magicor.conf

Builds cleanly in mock. I'll do the full review after I've had some lunch!

Comment 2 Paul F. Johnson 2006-10-17 21:40:41 UTC

Sorry for the delay on this review

Good

Docs included
Upstream and current md5sums check
Consistent use of macros
Software installs and runs as it should
No dependancy and provides problems (sane)
Compliant with the python packaging rules
Correct use of scriptlets
Has a desktop icon
spec file in American English and is sane

needs work
Under %files
non-conffile-in-etc /etc/magicor.conf - needs %config (noreplace) before the
%{_sysconfdir)
needs a %build section (even if it's empty - I'll need to check on this though)

Fix the %config bit under needs work and I'm happy to let it in.

Comment 3 Wart 2006-10-19 23:26:03 UTC

(In reply to comment #2)
> needs a %build section (even if it's empty - I'll need to check on this though)

I had a similar problem in one of my packages.  It was recommended to me on
#fedora-extras to just put in an empty %build section because there may be
problems in certain (unspecified) situations if %build was missing.  I usually
do the following to make it clear to spec file readers:

%build
# Nothing to build!

Comment 4 Hans de Goede 2006-10-21 19:57:27 UTC

(In reply to comment #2)

> needs work
> Under %files
> non-conffile-in-etc /etc/magicor.conf - needs %config (noreplace) before the
> %{_sysconfdir)

Fixed

> needs a %build section (even if it's empty - I'll need to check on this though)
> 

For packages which don't need any building I always get comments on how I handle
it if I add a comment that there is nothing to build, but leave %build
uncommented, people say that I should comment it (thats how/when I started
commenting %build itself). And if I do comment it I get remarks too.

Sofar I have several packages with %build commented and they all work fine, so I
would rather leave this as is.

---

Here is a new version with the %config(noreplace) added:
Spec URL: http://people.atrpms.net/~hdegoede/magicor.spec
SRPM URL: http://people.atrpms.net/~hdegoede/magicor-0.1-2.fc6.src.rpm

Comment 5 Paul F. Johnson 2006-10-21 20:02:17 UTC

Not a problem with the %build and as there isn't anything (that I can spot) in
the packaging guidelines, I'm happy with what you've said.

The new package builds and executes fine.

APPROVED

Comment 6 Hans de Goede 2006-10-21 20:48:37 UTC

Thanks!

Imported and build, closing.