Spec URL: http://dl.free.fr/jLC82iDWw/wastesedge.spec SRPM URL: (this is a french host server, click on "Télécharger ce fichier" to download the files) Description: As a loyal servant of the elven Lady Silverhair, you arrive at the remote trading post of Waste's Edge, where she is engaged in negotiations with the dwarvish merchant Bjarn Fingolson. But not all is well at Waste's Edge, and soon you are confronted with circumstances that are about to destroy your mistress' high reputation. And you are the only one to avert this ... Notes: 1. This is my first package submission (along with the one in bug 441411). 2. This package depends on adonthell, which I submitted in bug 441411.
Sorry, I forgot the URL for the SRPM. Here it is: http://dl.free.fr/oTkAOMwRF/wastesedge-0.3.4-0.5.fc8.src.rpm
I've been told in bug 441411 that the web host I'm using is making things difficult for people to dwonload the files. Here they are, more easily accessible. SPEC: http://fedora.c-bien.fr/wastesedge.spec SRPM: http://fedora.c-bien.fr/wastesedge-0.3.4-0.5.fc8.src.rpm
Hi Mathieu, Short intro: I'm a long time Fedora contributer and a sponsor. I haven't been sponsoring anyone in a while as I've been rather busy with other stuff. But getting new people into Fedora is important so I've decided to make some time for sponsering again. Most of the things I do within Fedora are gaming related so I'm very happy to see adonthell getting packaged. As such I would like to sponsor you. The first step here would be to finish up the packaging of adonthell and wastesedge so that they are in a state where they can be approved. The next step then is for you to either package one or two more packages, for example from these lists: http://fedoraproject.org/wiki/SIGs/Games/WishList http://fedoraproject.org/wiki/PackageMaintainers/WishList Or you can do some reviews of other people packages, when you do this please add a note that it is not an official review as you aren't a contributer yet. The purpose here is for you to show a good understanding of the packaging guidelines. Once you've done a couple of good reviews and / or submitted one or two more good packages, then you can apply for cvsextras membership in the account system and I'll sponsor you. I'll look into reviewing this once adonthell is in an approvable state.
I reworked it a little, trying to take into accounts remarks that were made to me in adonthell review. SPEC: ftp://91.121.156.173/pub/wastesedge.spec SRPM: ftp://91.121.156.173/pub/wastesedge-0.3.4-0.5.fc9.src.rpm
My bad, I uploaded the old SRPM. Here is the good one: SRPM: ftp://91.121.156.173/pub/wastesedge-0.3.4-0.6.fc9.src.rpm Spec file was right though, but it's always better to have the two of them in the same comment: SPEC: ftp://91.121.156.173/pub/wastesedge.spec
Full review done, results: Must Fix -------- * The GPL version to use does not get specified anywhere, so the correct License tag is GPL+ not GPLv2+ * Drop the obsolete Encoding and Version entries from the .desktop file Should Fix ---------- * Drop the #%files line (why is that there anyways?) * Add INSTALL="install -p" as argument to make install to preserve the timestamps of the files getting installed * Install icons in freedesktop.org icon standard location (/usr/share/pixmaps is deprecated) : mv wastesedge_16x16.xpm /usr/share/icons/hicolor/16x16/apps/wastesedge.xpm mv wastesedge_32x32.xpm /usr/share/icons/hicolor/32x32/apps/wastesedge.xpm Add a "Requires: hicolor-icon-theme" for icon dirs ownership Add ican-cache update scriptlets from: http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GTK.2B_icon_cache Change Icon line in .desktop to just "Icon=wastesedge" One all that is done its time for the next step as described in my initial comment: "The next step then is for you to either package one or two more packages, for example from these lists: http://fedoraproject.org/wiki/SIGs/Games/WishList http://fedoraproject.org/wiki/PackageMaintainers/WishList Or you can do some reviews of other people packages, when you do this please add a note that it is not an official review as you aren't a contributer yet. The purpose here is for you to show a good understanding of the packaging guidelines. Once you've done a couple of good reviews and / or submitted one or two more good packages, then you can apply for cvsextras membership in the account system and I'll sponsor you." So I would like you to either create one more package, or do 2 good reviews of other peoples submissions.
About the icons location, I looked for it so I could have discuss this upstream, and I found this in the Freedektop Icon Theme Spec (http://standards.freedesktop.org/icon-theme-spec/latest/ar01s03.html): > Icons and themes are looked for in a set of directories. By default, apps > should look in $HOME/.icons (for backwards compatibility), in > $XDG_DATA_DIRS/icons and in /usr/share/pixmaps (in that order). Are you sure it has been deprecated ? Do you have another link explaining it so I can sell this point upstream for future release ? Another question, you said: > mv wastesedge_16x16.xpm /usr/share/icons/hicolor/16x16/apps/wastesedge.xpm > mv wastesedge_32x32.xpm /usr/share/icons/hicolor/32x32/apps/wastesedge.xpm So I assume I should do that in a %post section, is that the preferred method over simply patching the Makefile so that it installs in the right place ? I corrected the "install -p" point, but you didn't make this remark in my other submission. Is there a reason why I should do this in this package and not in the other ? Or should I correct it in adonthell too ?
(In reply to comment #7) > About the icons location, I looked for it so I could have discuss this upstream, > and I found this in the Freedektop Icon Theme Spec > (http://standards.freedesktop.org/icon-theme-spec/latest/ar01s03.html): > > Icons and themes are looked for in a set of directories. By default, apps > > should look in $HOME/.icons (for backwards compatibility), in > > $XDG_DATA_DIRS/icons and in /usr/share/pixmaps (in that order). > > Are you sure it has been deprecated ? Do you have another link explaining it so > I can sell this point upstream for future release ? > Well I don't have a document saying so, but /usr/share/pixmaps is the old way of installing icons. It does not allow for theming, nor does it allow installing multiple sizes of an icon in such a way that a using application (like the gnome/kde/xfce applications menu) can automatically use the correct size depending upon the size it needs. With the /usr/share/icons way, if you have a large panel and place a launcher there you will automatically get the more detailed 32x32 icon, with the /usr/share/pixmaps way you are stuck to whatever size you've hardcoded in the .desktop file (as the size now is encoded into the name you've specified in the .desktop file). But as said this is a should, so if you feel this is not worth the trouble you do not have todo this. > Another question, you said: > > mv wastesedge_16x16.xpm /usr/share/icons/hicolor/16x16/apps/wastesedge.xpm > > mv wastesedge_32x32.xpm /usr/share/icons/hicolor/32x32/apps/wastesedge.xpm > > So I assume I should do that in a %post section, is that the preferred method > over simply patching the Makefile so that it installs in the right place ? > Indeed it is not necessary to patch the makefiles, but using things like mv in %post is a really bad idea, just move the files to the new location in $RPM_BUILD_ROOT in %install (after calling "make install") > I corrected the "install -p" point, but you didn't make this remark in my other > submission. Is there a reason why I should do this in this package and not in > the other ? Or should I correct it in adonthell too ? You should correct it in adonthell too, I realised this myself too when making the remark here, but with adonthell its less important as AFAIK no datafiles are installed there, only compiled files which will have the time of build as timestamp anyways, so preserving there timestamp is not really important, but for autoxxx generated makefiles you should really _always_ specify INSTALL="install -p"
Corrected the 5 points. Here are the new files. SRPM: ftp://91.121.156.173/pub/wastesedge-0.3.4-0.7.fc9.src.rpm SPEC: ftp://91.121.156.173/pub/wastesedge.spec
Just had a revelation: I forgot the icon-cache update scriptlet :S I'm terribly sorry about that. I'll update this tonight (in France) as I can't rebuild it from work.
Ok, I'll wait for the next version then.
Sorry for forgetting something that was written black on white here -_- SPEC: ftp://ks359320.kimsufi.com/pub/wastesedge.spec SRPM: ftp://ks359320.kimsufi.com/pub/wastesedge-0.3.4-0.8.fc9.src.rpm
Sorry for the long delay, I git hit by a wall / mountain of work, but I've managed to get over the top now :) Anyways this package is approved! Given the good work you've done here I'm ready to sponsor you now. Head over the the fedora account system, and apply for cvs extra membership there. Once you've done that add a comment here where you tell me your fas username, then I can sponsor you.
Hi, Thanks a lot for the review. I just applied for cvsextras in FAS, my user name is bochecha.
Ok, I've sponsored you, now you can do yhe cvs requests.
New Package CVS Request ======================= Package Name: wastesedge Short Description: Official game package for Adonthell Owners: bochecha Branches: F-8 F-9 InitialCC: Cvsextras Commits: yes
cvs done.