Bug 210757 - Review Request: magicor - Push ice blocks around to extenguish all fires
Review Request: magicor - Push ice blocks around to extenguish all fires
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul F. Johnson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-10-14 07:48 EDT by Hans de Goede
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-10-21 16:48:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2006-10-14 07:48:43 EDT
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 08:27:07 EDT
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 17:40:41 EDT
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 19:26:03 EDT
(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 15:57:27 EDT
(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 16:02:17 EDT
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 16:48:37 EDT
Thanks!

Imported and build, closing.

Note You need to log in before you can comment on or make changes to this bug.