Bug 210757

Summary: Review Request: magicor - Push ice blocks around to extenguish all fires
Product: [Fedora] Fedora Reporter: Hans de Goede <hdegoede>
Component: Package ReviewAssignee: Paul F. Johnson <paul>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: 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: ---
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.