Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 199611 - Review Request: monsterz - Puzzle game, similar to Bejeweled or Zookeeper
Summary: Review Request: monsterz - Puzzle game, similar to Bejeweled or Zookeeper
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michał Bentkowski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-07-20 19:06 UTC by Ian Chapman
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-07-27 22:52:25 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ian Chapman 2006-07-20 19:06:33 UTC
Spec URL: http://dribble.org.uk/reviews/monsterz.spec
SRPM URL: http://dribble.org.uk/reviews/monsterz-0.7.0-4.src.rpm
Description: 

Monsterz is a little arcade puzzle game, similar to the famous Bejeweled or
Zookeeper. The goal of the game is to create rows of similar monsters, either
horizontally or vertically. The only allowed move is the swap of two adjacent
monsters, on the condition that it creates a row of three or more. When
alignments are cleared, pieces fall from the top of the screen to fill the
board again. Chain reactions earn you even more points.

Comment 1 Michał Bentkowski 2006-07-23 09:08:10 UTC
I'll review this package, but before doing it you have to do some small fixes
in your SPEC file.

 * change License field from Freeware to WTFPL
 * you should include monsterz.score and monster.desktop as another sources,
do not create it in SPEC file
 * please read http://fedoraproject.org/wiki/Extras/SIGs/Games#head-
177dfd094f552a49a7b33ae4c65894ffac8f7853,
according to it, you should change game directory from %{_datadir}/games/%{name}
to %{_datadir}/%{name}

Comment 2 Ian Chapman 2006-07-23 22:40:43 UTC
Spec URL: http://dribble.org.uk/reviews/monsterz.spec
SRPM URL: http://dribble.org.uk/reviews/monsterz-0.7.0-5.src.rpm

Thanks, this should fix those issues.

Comment 3 Michał Bentkowski 2006-07-24 11:17:13 UTC
You have to do one more thing. http://fedoraproject.org/wiki/Extras/SIGs/Games
also says that data files should be packaged seperately of main package.
Thus make new -data subpackage and include into it %{_datadir}/%{name} and, for
the main package, make monsterz-data dependency. Take a look on
http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/tong/FC-5/
tong.spec?root=extras if you would like to see how should it looks like.

Comment 4 Wart 2006-07-24 21:44:49 UTC
(In reply to comment #3)
> You have to do one more thing. http://fedoraproject.org/wiki/Extras/SIGs/Games
> also says that data files should be packaged seperately of main package.
> Thus make new -data subpackage and include into it %{_datadir}/%{name} and, for
> the main package, make monsterz-data dependency. Take a look on
> http://cvs.fedora.redhat.com/viewcvs/*checkout*/rpms/tong/FC-5/
> tong.spec?root=extras if you would like to see how should it looks like.

It's more important to separate the two when upstream 

Comment 5 Wart 2006-07-24 22:00:21 UTC
Ignore my previous comment.  I hit 'submit' too soon.

Anyway, you might also want to use 'install -p' to preserve the timestamps on
the source files.

Comment 6 Wart 2006-07-24 22:16:38 UTC
I'm also concerned about the setgid scoreboard handling.  To my untrained eye,
it looks secure.  But there might be concurrency issues if multiple users try to
write to the file at the same time.  I don't see any sort of locking to ensure
synchronized access, but then again, the section of code that actually writes to
the file is pretty isolated and short-lived, so it might not be enough to worry
about.

Comment 7 Ian Chapman 2006-07-25 18:42:20 UTC
Spec URL: http://dribble.org.uk/reviews/monsterz.spec
SRPM URL: http://dribble.org.uk/reviews/monsterz-0.7.0-6.src.rpm

Here's the latest version. 

It looks secure to me, but I'm also an untrained eye. I agree, there appears to 
be no file locking although the example on the wiki doesn't seem to imply this 
either. I think the possibility of corrupting the scoreboard file is extremely 
small but that's just IMHO. :-)

Comment 8 Michał Bentkowski 2006-07-25 19:59:50 UTC
MUST items:
 * rpmlint output:
I: monsterz checking
W: monsterz invalid-license WTFPL
E: monsterz non-standard-executable-perm /usr/bin/monsterz 02755
I: monsterz-data checking
W: monsterz-data invalid-license WTFPL
W: monsterz-data no-documentation
I: monsterz-debuginfo checking
W: monsterz-debuginfo invalid-license WTFPL
 despite error, permissions of /usr/bin/monsterz are good
 * package is named good
 * spec file name is correct
 * package meets Packaging Guidelines
 * package is licensed with an open-source compatible license
 * License field in .spec file matches the actual license (WTFPL)
 * spec file is written in American English and is legible
 * md5sum of upstream source and provided in SRPM is matching
(323d04d4a2a2905df91eab4ff17e537d)
 * package succesfully compile on i386
 * dependencies are good
 * there are no locales
 * no need to /sbin/ldconfig
 * there is no duplicate files in %files
 * spec file hadnles macros properly
 * there is no need to -devel subpackage
 * package doesn't contatin any .la files
 * desktop file is created properly

And the package compiles successfully on mock.

> I think the possibility of corrupting the scoreboard file is extremely 
> small but that's just IMHO. :-)

IMHO, too ;)

Approved.
 



Comment 9 Michał Bentkowski 2006-07-27 21:39:27 UTC
Ian, you have imported package on CVS, but why haven't you built it?

Comment 10 Ian Chapman 2006-07-27 22:36:56 UTC
I was waiting for the additional branches :-) It's building now.


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