Spec Name or Url: http://home.zonnet.nl/jwrdegoede/shippy.spec SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/shippy-1.3.3.7-1.src.rpm Description: Shippy1984 is a small, portable game designed to bring back nostalgia for the ways games used to be made--addicting as hell! Mash buttons on your way to the high score! Shippy1984 is designed from the ground up for the avid casual gamer who feels left behind by the technological overload of today's games! No longer! Shippy1984 is the game you have been waiting for!
Two minute review: -BR is missing allegro-devel -rpmlint errors/warnings: W: shippy no-documentation E: shippy non-standard-executable-perm /usr/bin/shippy 02755 W: shippy-allegro no-documentation E: shippy-allegro non-standard-executable-perm /usr/bin/shippy-allegro 02755 I guess these are okay to be ignored. I tried running shippy and fullscreen mode does not work properly on my LCD monitor. The screen is shifted to the right and down so I only see the upper-left portion of the screen. Probablly due to a bad X11 config on my part. -package name okay -If I only install shippy-allegro I do not get a desktop entry -Shouldnt shippy-data be noarch? -Could not find: http://download.sourceforge.net/ship84/shipv1.3.3.7UNIX.zip -Could not verify source due to error above -Illegal use of elite version numbers -The angry Fez won!
(In reply to comment #1) > Two minute review: > > -BR is missing allegro-devel ditto. > -rpmlint errors/warnings: > W: shippy no-documentation Perhaps the docs from shippy-data should move to the base shippy package. rpmlint will warn about no docs for shippy-data, but that's fine. > E: shippy non-standard-executable-perm /usr/bin/shippy 02755 > W: shippy-allegro no-documentation Either make shippy-allegro require shippy, or duplicate the docs in shippy-allegro. Based on my .desktop file comments below, I would recommend the former > E: shippy-allegro non-standard-executable-perm /usr/bin/shippy-allegro 02755 This is allowed per the Games SIG packaging guidelines for shared scoreboard files. > I guess these are okay to be ignored. I tried running shippy and fullscreen > mode does not work properly on my LCD monitor. The screen is shifted to the > right and down so I only see the upper-left portion of the screen. Probablly > due to a bad X11 config on my part. > > -package name okay > -If I only install shippy-allegro I do not get a desktop entry Since there are two executables (shippy shippy-allegro), I would suggest changing the name of the executable 'shippy' to 'shippy-std' and providing a wrapper script that invokes shippy-allegro, if present, or shippy-std if not. This wrapper should be in the base 'shippy' package and called by the .desktop file. This will allow the .desktop file to launch the best available frontend found, but would also force shippy-allegro to Requires: shippy. > -Shouldnt shippy-data be noarch? It should, but you can't provide arch and noarch packages from a single spec file. Unless upstream provides a separate tarball for the -data files, the use of a single spec that generates an arch-specific -data package is fine. > -Could not find: > http://download.sourceforge.net/ship84/shipv1.3.3.7UNIX.zip > -Could not verify source due to error above > -Illegal use of elite version numbers ?? As far as I'm concerned, if upstream uses 1.3.3.7 as the version number, the spec file should as well. I wonder what they'll change it to if they ever release a new version... > -The angry Fez won! No kidding! Curse the angry Fez!
> I would suggest changing the name of the executable 'shippy' to > 'shippy-std' and providing a wrapper script that invokes > shippy-allegro, if present, or shippy-std if not. Or perhaps make a shippy-allegro, shippy-sdl, and shippy, where shippy is the wrapper script that calls which ever version it finds. >> -Illegal use of elite version numbers > ?? As far as I'm concerned, if upstream uses 1.3.3.7 as the version number Yes, I just wanted the point out the humor in the version number incase anyone else missed it ;-)
About the missing BR allegro-devel, no it is not dumb-devel requires allegro-devel. Erm that is dumb-devel should require allegro-devel. I'll fix this in dumb right away. About the 2 versions mess, I suggest (and lets make this game SIG policy for similar cases, which will happen): -rename shippy-data to shippy-common leave the docs there -put a wrapper which wille executa any shippy found prefering the SDL version in shippy-common -mv desktop and icon to shippy-common, make desktop call the wrapper -make -common require shippy-engine -make both shippy and shippy-allegro provide shippy-engine and require shippy-common -a regular user and/or one using comps will install shippy which will suck in -common, result everything ok -someone who want the allegro version can install shippy-allegro which will suck in -common, result everything ok -someone who insists on doing a "yum install shippy-common" will get shippy (shortest name) resulting in a working setup too. Please ack if this is ok, then I can make it so.
About the not showing properly on an LCD thats my fault. I've modified it so that it starts fullscreen by default (it used to be in a window and only in a window) I've also made it so that alt+enter toggles fullscreen<->window. The LCD problem is because by default it uses (I didn't change this part) a funky resolution of 480x320 (which gives nice oldscool scanlines on my CRT). I've uploaded a new SRPM (sorry I didn't bump release) with an extra patch which does the following: -make the default res 640x480 -adds a -w cmdline option to start in a window instead of fullcreen -adds a -a (arcade) option to get the 480x320 resolution instead of 640x480 -allegro-version: be nice (sleep when idle) (sdl already had this). So this new SRPM should (once compiled) fix things on your LCD. New version (same location as previous) at: Spec Name or Url: http://home.zonnet.nl/jwrdegoede/shippy.spec SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/shippy-1.3.3.7-1.src.rpm
tkmame, adding you to CC because you seem interested and didn't do so yourself, please read my previous comments (replies to your comments).
Michael, adding you to CC because you seem interested and didn't do so yourself, please read my previous comments (replies to your comments).
(In reply to comment #7) > Michael, adding you to CC because you seem interested and didn't do so yourself, > please read my previous comments (replies to your comments). Oops. You're right, I forgot to cc myself. I've been following the thread in f-e-l, though. tkmame was the first to comment on this package, so I'll let him take the review if he wants it. Otherwise I'll assign it to myself.
(In reply to comment #8) > tkmame was the first to comment on this package, so I'll let him take the review > if he wants it. Otherwise I'll assign it to myself. > Ok, Any chance you could comment on comment 4, this needs some kinda concensus before I can move forward.
(In reply to comment #4) > About the 2 versions mess, I suggest (and lets make this game SIG policy for > similar cases, which will happen): > -rename shippy-data to shippy-common leave the docs there > -put a wrapper which wille executa any shippy found prefering the SDL version > in shippy-common > -mv desktop and icon to shippy-common, make desktop call the wrapper > -make -common require shippy-engine > -make both shippy and shippy-allegro provide shippy-engine and require > shippy-common > -a regular user and/or one using comps will install shippy which will suck in > -common, result everything ok > -someone who want the allegro version can install shippy-allegro which will suck > in -common, result everything ok > -someone who insists on doing a "yum install shippy-common" will get shippy > (shortest name) resulting in a working setup too. > > Please ack if this is ok, then I can make it so. This sounds fine to me. Can you generalize the rule and add it to the Games SIG page? This affects xpilot-ng, as it simply packages two .desktop files in the client package instead of using a wrapper script.
Hi, I can take the review, but I dont know if I have the authority to approve something yet. But I can atleast do the tedious dirty work for you. Hans: I tested your latest fullscreen patch and it works great with both SDL and allegro versions!
(In reply to comment #11) > Hi, I can take the review, but I dont know if I have the authority to approve > something yet. But I can atleast do the tedious dirty work for you. If you have already received sponsorship and approval for your first package then you have authority to approve other packages. I don't see you in my (2-week old) copy of owners.list, so you might not have approval rights yet. If that's the case then I'll take ownership of this, but I'll refrain from giving it a final approval until you say it's ok. :)
I just received sponsorship today, but not approval of my first package yet. I did talk to my sponsor and he said I can approve other packages, so I'll go ahead and review this one when it's ready.
(In reply to comment #10) > (In reply to comment #4) > > About the 2 versions mess, I suggest (and lets make this game SIG policy for > > similar cases, which will happen): > > This sounds fine to me. Can you generalize the rule and add it to the Games SIG > page? Sure will do. > This affects xpilot-ng, as it simply packages two .desktop files in the > client package instead of using a wrapper script. > I'm not sure if xpilot-ng is similar judging by the name and .desktop comments this really are 2 different versions. Whereas with shippy both version are 100% identical to the user except that the titlebar says SDL or allegro. The only difference between the 2 versions of shippy is the display library they use. I've also contemplated on just choosing one version and only including that, but hey choice is everything right?
About the broken download URL, it seems all download.sf.ne tis completly broken, any ideas?
If you're referring to the URL in comment 1, it works for me in Firefox. Wget seems to be eventually (I have timeout = 10 in ~/.wgetrc) redirected to belnet.dl.sf.net which doesn't work, but heanet.dl.sf.net does.
I've uploaded a new version, doing "things" as described in comment 4. I didn't know what to put in a changelog entry for all these changes, so I just left the release field at 1 :) Go get it (and review it) from: Spec Name or Url: http://home.zonnet.nl/jwrdegoede/shippy.spec SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/shippy-1.3.3.7-1.src.rpm
New version: * Thu Mar 30 2006 Hans de Goede <j.w.r.degoede> 1.3.3.7-2 - Fix reversed joy directions in allegro version - Improve support for analog joysticks in the allegro version - Add support for joysticks to the SDL version Go get it (and review it) from: Spec Name or Url: http://home.zonnet.nl/jwrdegoede/shippy.spec SRPM Name or Url: http://home.zonnet.nl/jwrdegoede/shippy-1.3.3.7-2.src.rpm
I haven't seen any comments from tkmame lately, so here's a full review. tkmame: please assign this to yourself if you would still like to give the final approval. MUST ==== rpmlint warnings: W: shippy no-documentation W: shippy-allegro no-documentation Perhaps put at least the license file in each of these? The rest of the docs are in shippy-common, which is fine. E: shippy non-standard-executable-perm /usr/bin/shippy-sdl 02755 E: shippy-allegro non-standard-executable-perm /usr/bin/shippy-allegro 02755 This is allowed per the Games SIG guidelines for shared scoreboard files. E: shippy-common score-file-must-not-be-conffile /var/lib/games/shippy.hs This will go away if you move it to /var/games per the FHS. E: shippy-common zero-length /var/lib/games/shippy.hs Empty initial scoreboard file. This is fine. * Package and spec named appropriately * License (GPL) ok, license file included * Spec file legible, in Am. English * Source matches upstream 06df2ae060fe4a076d7fa17a57205348 shipv1.3.3.7UNIX.zip * compiles and builds on FC5 i386 * No excessive or offensive BR: * No locales * No shared libraries * Not relocatable * Owns directories that it creates (/usr/share/shippy) * Permissions look ok. setgid binary acceptable (see rpmlint warnings above) * %install and %clean both clean $RPM_BUILD_ROOT * %doc does not affect runtime * Contains code and permissible content * No -devel package * .desktop file included and installed properly * Runs without crashing RECOMMENDED =========== * Compiler warning: shipall.c: In function 'SYSTEM_INIT': shipall.c:264: warning: 'set_window_close_hook' is deprecated (declared at /usr/include/allegro/alcompat.h:198) This is probably safe for now, but you might want to consider not using the deprecated function to avoid problems with future versions of allegro that might remove it. * Move the high score file from /var/lib/games to /var/games per FHS. This will also clean up one of the rpmlint warnings. I don't consider either of the RECOMMENDED items blockers, but it would be nice if Christopher could verify the joystick patch (no game port on my desktop, unfortunately).
- MUST: rpmlint must be run on every package. The output should be posted in the review. W: shippy no-documentation OKAY: documentation is in shippy-common which is required by shippy E: shippy non-standard-executable-perm /usr/bin/shippy-sdl 02755 OKAY: needed to save high scores W: shippy-allegro no-documentation OKAY: documentation is in shippy-common which is required by shippy-allegro E: shippy-allegro non-standard-executable-perm /usr/bin/shippy-allegro 02755 OKAY: needed to save high scores E: shippy-common score-file-must-not-be-conffile /var/lib/games/shippy.hs ????: I have no idea what this means and rpmlint -I didn't have any idea either. E: shippy-common zero-length /var/lib/games/shippy.hs OKAY: Safe to ignore. package name OKAY spec file name OKAY package meets package guidelines OKAY package OSI license OKAY package matches license in source OKAY license in %doc OKAY spec file in english OKAY spec file legible OKAY sources match upstream sources OKAY package successfully builds on x86_64 for FC5 OKAY package does not contain any extra BRs OKAY all dependencies in BR OKAY [1] no locales in spec OKAY package does not contain shared libraries OKAY package is not relocatable OKAY package owns directories it creates OKAY package contains no duplicate files OKAY file permissions set correctly OKAY package contains good %clean section OKAY macro use is consistant OKAY pacakge contains permissible content OKAY package does not contain large docs OKAY %doc files do not affect runtime OKAY package does not contain any files that should go in -devel OKAY package does not contain any .la files OKAY package contains a %{name}.desktop OKAY package does not own files owned by other packages OKAY One other thing I noticed is when I try to install shippy-common I get: # rpm -ivh shippy-common-1.3.3.7-2.x86_64.rpm error: Failed dependencies: shippy-engine = 1.3.3.7 is needed by shippy-common-1.3.3.7-2.x86_64 however, there is no shippy-engine package, just shippy. APPROVED! [2][3] [1] dumb-devel requires allegro-devel must be fixed! This still fails in mock for FC5. [2] Since this is my first ever official approval perhaps wart or someone might want to take a quick 2nd look [3] Please review that rpmlint error which I couldn't understand, and look into why installing shippy-common asks for shippy-engine when the package name is just "shippy". Whew! P.S. Joystick works great in both versions now!
Seems we did the reviews at the same time. I tried to assign this bug to me, but I do not see where this is done? Perhaps I don't have permissions yet to assign myself to bugs? Not sure why I can't do that yet.
(In reply to comment #20) > One other thing I noticed is when I try to install shippy-common I get: > # rpm -ivh shippy-common-1.3.3.7-2.x86_64.rpm > error: Failed dependencies: > shippy-engine = 1.3.3.7 is needed by shippy-common-1.3.3.7-2.x86_64 > however, there is no shippy-engine package, just shippy. shippy-engine is provided by both the base shippy package and shippy-allegro. You need to install shippy-common with one of these to get the full game. Fortunately, yum will make sure that the dependencies are satisfied. > APPROVED! [2][3] > > [1] dumb-devel requires allegro-devel must be fixed! This still fails in mock > for FC5. If I'm not mistaken, Hans has (or is in the process of) fixing this in the dumb package. > [2] Since this is my first ever official approval perhaps wart or someone might > want to take a quick 2nd look Already did. :) > [3] Please review that rpmlint error which I couldn't understand, rpmlint does not like high score files in /var/lib/games being marked as configuration files. Perhaps we need to discuss this in the Games SIG. Should scoreboard files be marked %config(noreplace) or not? They aren't really configuration files, but we also don't want to have them replaced during an upgrade. > and look into > why installing shippy-common asks for shippy-engine when the package name is > just "shippy". See comment 4 for the explanation of shippy-engine.
(In reply to comment #21) > Seems we did the reviews at the same time. I tried to assign this bug to me, > but I do not see where this is done? Perhaps I don't have permissions yet to > assign myself to bugs? Not sure why I can't do that yet. There is a "Bug Reassignment" area at the bottom of the bug page. Enter your email address next to "Reassign bug to ___" and hit "Save Changes". It seems that Ignacio has done this for you already. :) One more thing: Once you APPROVE a package, you should change the blocking bug from FE-REVIEW to FE-ACCEPT.
Thanks for the reviewS guys! Michael, thanks for answering most of Christophers (tkmame's) questions. About dumb-devel, this is already fixed in devel, but not for FC-5, I'll fix this for FC-5 too, good piont. About the score file should not be config warning I took a look how gnome-games handles this and it uses: %config(noreplace) %attr(664, games, games) /var/lib/games/* So I guess this is ok and the rpmlint warning should be removed or ignored. This still leaves the /var/games vs /var/lib/games issue, see bug 165425 . I'm sticking with /var/lib/games for now as thats in filesystem, this will however cause migration issues when moving to /var/games <sigh> .
Okay, sounds good. Moving bug over to FE-ACCEPT.
Imported and Build, thanks!