Red Hat Bugzilla – Bug 235588
Review Request: escape - an extensible puzzle game
Last modified: 2007-11-30 17:12:01 EST
Spec URL: http://www.spicenitz.org/fedora/escape.spec SRPM URL: http://www.spicenitz.org/fedora/escape-200704070-1.fc7.src.rpm Description: Escape is a tile-based puzzle game in the style of "Adventures of Lolo" or "Chip's Challenge." Unlike either of those games, Escape doesn't rely at all on reflexes--it's all about your brain. Although Escape comes with hundreds of levels, the game places an emphasis on the composition of new puzzles. Thus Escape has a built-in level editor and facilities for automatically sharing puzzles with other players.
This is mutually dependent on escape-data, also pending review.
Wiki is down, but this is an initial list of issues: escape owns all of %{bindir}. Should only own what it puts there. Remove X-Fedora category Icon-cache is not updated on install and remove, see the wiki when it's back up. Source0 should include the full download URL. Builds in mock, though. I'll peek at -data.
You should also add hicolor-icon-theme to Requires. This is an empty package owning the /usr/share/icons/hicolor dir hierarchy, sortoff like the filesystem package but then for icons. Note that all packages which have an icon should Require this to ensure proper icon dir ownership. Version is wrong. Please read: http://fedoraproject.org/wiki/Packaging/NamingGuidelines?action=show#head-cfd71146dbb6f00cec9fe3623ea619f843394837 BTW, why do you use a snapshot version (CVS checkout) instead of a stable release? There may be good reasons, but I'd like to know :)
(In reply to comment #3) > BTW, why do you use a snapshot version (CVS checkout) instead of a stable > release? There may be good reasons, but I'd like to know :) > +1. Would probably also make the URL issue go away more easily.
New release: http://www.spicenitz.org/fedora/escape-200704130-1.fc7.src.rpm http://www.spicenitz.org/fedora/escape.spec - Merge -data package into this - Install icon into correct place and update the icon cache - Use upstream tarball, created by upstream at my request - Remove X-Fedora category
Backup location if spicenitz.org is down: http://www.andrew.cmu.edu/user/agoode/fedora/
Ok, let's see. rpmlint -i clean, except for debuginfo, which complains that several source files are marked executable. Might want to submit to upstream, but not a showstopper IMHO: W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/dirt.h W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound.h E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/dirt.cpp E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/sound.cpp W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/progress.h W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound_enum.h E: escape-debuginfo script-without-shebang /usr/src/debug/escape-src/progress.cpp Good package/spec names. Meets packaging guidelines. License OK. Spec is legible American English. [limb@fawkes fedora]$ md5sum escape-src-200704130.tar.bz2 07263976a54607792dbbe4bc442f0735 escape-src-200704130.tar.bz2 [limb@fawkes fedora]$ md5sum /usr/src/redhat/SOURCES/escape-src-200704130.tar.bz2 1764ef8aad634e6f71c5533fc7eafe21 /usr/src/redhat/SOURCES/escape-src-200704130.tar.bz2 ISSUE: MD5 mismatch. Put the tarball from upstream straight in the SRPM, no modification.
If I may add, even if I'm not the reviewer, the version is wrong. Using the release or snapshot date as the version is going to cause you trouble when version x.y will be released. E.g. 200704130 > 1.0. If a stable release is not yet planned upstream, and therefore it is not possibile to identify this as a prerelease of a planned release, I would suggest to use 0.0 as version and 0.release.200704130pre as the release. For a complete overview, please read from the following link onwards. http://fedoraproject.org/wiki/Packaging/NamingGuidelines?action=show&redirect=PackageNamingGuidelines#head-18aa467fc6925455e44be682fd336667a17e8933
Good point. What are upstream's plans? If they never plan on a sane versioning system 2007x would work, but even so I agree with Andrea.
I'll have to look into the MD5 mismatch thing later tonight. Something weird has happened. As for the version, upstream has no plans to move from date-based version number. So for now the version number is correct. If upstream changes its scheme, that's what epoch is for.
http://www.spicenitz.org/fedora/escape-200704130-2.fc7.src.rpm http://www.spicenitz.org/fedora/escape.spec - Fix permissions in debuginfo package - Generate SRPM with source matching upstream MD5
MD5 matches. Still some -debug perm issues. W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/dirt.h W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound.h W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/progress.h W: escape-debuginfo spurious-executable-perm /usr/src/debug/escape-src/sound_enum.h
Jon, you should set the fedora review flag to ? when doing a review.
Whoops. Thanks. :)
New version: http://www.spicenitz.org/fedora/escape.spec http://www.spicenitz.org/fedora/escape-200704130-3.fc7.src.rpm Changes: - REALLY fix spurious-executable-perm
(In reply to comment #15) > > - REALLY fix spurious-executable-perm > Confirmed. :) Builds on FC6 1386. BRs are good. No shared libs. Not relocatable. ISSUE: Ownership is OK, except for the .desktop line. It should own it's own desktop file, not all of them (*.desktop).
What's wrong with having something like this? %files %defattr(-,root,root,-) %doc COPYING design.txt escape.txt README %{_bindir}/* %{_datadir}/%{name} %{_datadir}/icons/hicolor/32x32/apps/*.png %{_datadir}/applications/*.desktop The * doesn't mean I own everything, just the files that I installed to $RPM_BUILD_ROOT. This can be verified using rpm -ql escape.
Oh, so it is. I didn't know that. Nevermind. :) No duplicate files. Permissions are ok. %clean present and correct. Macros OK. Code, not content. No huge docs. No runtime doc issues. No shared or static libs, or headers. No pkgconfig files. No devel package. No .la. .desktop file handled properly. No cross-ownership. %install starts right. Good filenames. All MUSTS met. Builds in mock of FC-6 i386. Runs as advertised. APPROVED.
New Package CVS Request ======================= Package Name: escape Short Description: an extensible puzzle game Owners: adam@spicenitz.org Branches: FC-5 FC-6 InitialCC: