Spec URL: http://dribble.org.uk/reviews/asylum.spec SRPM URL: http://dribble.org.uk/reviews/asylum-0.2.2-1.src.rpm Description: SDL Asylum is a C port of the computer game Asylum, which was written by Andy Southgate in 1994 for the Acorn Archimedes and is now public domain. The object is to find things that look like brain cells and shut them down! The game revolves around shooting anything which moves, collecting anything which doesn't move and most importantly, finding your way to each of the eight pulsating neurons scattered through the immense map. Note: It is currently excludearch ppc/ppc64 probably because of many endian issues. It's also setgid games because of the global hiscore table.
Just working through the review now and will post when done, but one issue crops up. The upstream name is sdl-asylum rather than asylum. On the one hand renaming the package to sdl-asylum would keep it inline with upstream and distinguished from the original Archimedes game of the same name. On the other hand it would at a glance appear to be an sdl addon package, although true sdl addons would use the underscore so the likelyhood of a name collision with an actual sdl addon is not an issue. What is the rationale behind the name choice?
(In reply to comment #1) > The upstream name is sdl-asylum rather than asylum. The upstream name is asylum, the project name is sdl-asylum > What is the rationale behind the name choice? rpm name usually follow upstream names where practical, of course there's a whole host of exceptions but I don't think this falls into one. Secondly, as you mentioned calling it sdl-asylum made be a little confusion so IMHO asylum is correct.
OK - Meets Packaging Guidelines. OK - Package named correctly OK - Patches named correctly OK - Spec file named correctly to match base OK - License is valid OK - Licence field matches package OK - Licence file installed if supplied OK - Spec file in American English OK - Source matches upstream (md5) e349a99bf099df818b0efc4f83359858 NA - Locales use %find_lang No locales OK - %clean is present and correct OK - Package has correct buildroot. OK - Specfile Legible OK - Builds in Mock F7-i386 NA - %post/%postun calls ldconfig for sh libs No sh libs OK - Owns directories it creates OK - No duplicate files OK - Has %defattr and has correct permissions ?? *2 Macros used consistently OK - %doc does not affect runtime NA - Headers/static libs in -devel NA - .pc files in -devel NA - .so files in -devel NA - -devel requires base No devel OK - Contains no .la libtool archive files OK - Does not own others files OK - .desktop files installed correctly OK - BuildRequires correct. OK - Package is code or permissible content. OK - Package has rm -rf %{buildroot} at top of %install. OK - Package compiles and builds on at least one arch. F7-i386 OK *1 rpmlint output. NA - documentation in -doc package OK - final provides and requires are sane. OK - should have dist tag OK - should package latest version Move %doc to top of %files to aid readability *1 asylum.i386: E: non-standard-executable-perm /usr/bin/asylum 02755 asylum.i386: E: non-standard-dir-perm /var/games/asylum 0775 Both due to global hi-score table as explained above. *2 Not a blocker but definitely a suggestion. Some path details are "hardcoded" into the Makefile and makefile patch. While the location of /usr/bin /usr/share/asylum and /var/games/asylum are not likely to change anytime soon it would be more visible to make these changes with sed in the specfile making use of the available macros.
(In reply to comment #3) > Move %doc to top of %files to aid readability I prefer %doc near the bottom, for me thats more readable. It suits the logical order that I write specs. Installed files, doc files, ghosted files. Unless there's a requirement to change it, I'd rather leave as-is. > Not a blocker but definitely a suggestion. > Some path details are "hardcoded" into the Makefile and makefile patch. > While the location of /usr/bin /usr/share/asylum and /var/games/asylum are not > likely to change anytime soon it would be more visible to make these changes > with sed in the specfile making use of the available macros. Well as I need to patch the Makefile anyway, I don't see a great deal of advantage, otherwise it would make sense. It just turns it from a 1 stage process to a 2 stage process.
well as I said, both are style choices. approved.
Thanks very much for the review.
New Package CVS Request ======================= Package Name: asylum Short Description: SDL port of the game Asylum, originally for the Archimedes Owners: oddsocks Branches: FC6 F7 InitialCC: <empty> Cvsextras Commits: Yes
cvs done.
Imported and built.