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 185257
Summary: | Review Request: raidem - 2d top-down shoot'em up | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Hans de Goede <hdegoede> | ||||||||||
Component: | Package Review | Assignee: | Wart <wart> | ||||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||||
Severity: | medium | Docs Contact: | |||||||||||
Priority: | medium | ||||||||||||
Version: | rawhide | CC: | wart | ||||||||||
Target Milestone: | --- | ||||||||||||
Target Release: | --- | ||||||||||||
Hardware: | All | ||||||||||||
OS: | Linux | ||||||||||||
Whiteboard: | |||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||
Doc Text: | Story Points: | --- | |||||||||||
Clone Of: | Environment: | ||||||||||||
Last Closed: | 2006-03-15 20:57:26 UTC | Type: | --- | ||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||
Documentation: | --- | CRM: | |||||||||||
Verified Versions: | Category: | --- | |||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
Embargoed: | |||||||||||||
Bug Depends On: | 185215, 185216 | ||||||||||||
Bug Blocks: | 163779 | ||||||||||||
Attachments: |
|
Description
Hans de Goede
2006-03-12 21:05:23 UTC
Michael, Me again this is the game for which I've been packaging adime and glyph-keeper. I know I still need to add a .desktop file, but its late over here now :) p.s. What do you think about my prboom comments? I made this bug dependent on glyph-keeper and adime. I'll try to get to it after I'm done reviewing glyph-keeper. I'd have done this sooner but I've been extensively "testing" prboom + freedoom today. :) I spazzed on setting the bug dependencies last time. This should fix it right up. Created attachment 126043 [details]
updated spec with desktop stuff
Hmm, this is becoming a mess, sorry. I really should get a decent ISP. I'm
moving at the end of this year, which will be a good moment for this.
Anyways attached is a new spec and I'll also add the .desktop and .png (ico)
file. As already explained I can't upload a srpm as my isp doesn't allow access
from anywhere but home and also it would be to big :(
Created attachment 126045 [details]
raidem desktop file
Created attachment 126052 [details]
raidem icon file
Instead of removing the duplicate libraries in the source (glyph-keeper, adime, etc.), the building of these libraries should just be disabled in the Makefile. This will help keep the source as close of a match to upstream as possible. The annoying -src in the tarball should remain as well. The mp3 encoder should still be rm'd, however. The license should also be "zlib License". I'll try to do a formal review later today. Unfortunatly its not that easy, not all libraries included can be disabled in the Makefile and because loadpng from the lib dir is actually needed -Ilib must be added, thus leaving these directories under lib can cause the header file under lib to be used instead of the system installed version . I could leave them in and rm them under %prep if you think thats preferable. About the license, thats already fixed in the attached updated version of the .spec with the desktopfile stuff added to it. (In reply to comment #8) > Unfortunatly its not that easy, not all libraries included can be disabled in > the Makefile and because loadpng from the lib dir is actually needed -Ilib must > be added, thus leaving these directories under lib can cause the header file > under lib to be used instead of the system installed version . > > I could leave them in and rm them under %prep if you think thats preferable. Yes, I think that would be better. > About the license, thats already fixed in the attached updated version of the > .spec with the desktopfile stuff added to it. My mistake. I was looking at the wrong spec file. Kind request, can you do the full review with the sources as is (iow using a soruce tarbell with the -src and lib dir stripped), then I can fix all problems in one go (I hope.) Thanks! rpmlint warnings: W: raidem invalid-license zlib License W: raidem-debuginfo invalid-license zlib License These can be ignored. This is an acceptible license. W: raidem uncompressed-zip /usr/share/raidem/data/wraith.zip W: raidem uncompressed-zip /usr/share/raidem/data/proj-maser.zip W: raidem uncompressed-zip /usr/share/raidem/data/sea-urchin.zip W: raidem uncompressed-zip /usr/share/raidem/data/conquerer.zip W: raidem uncompressed-zip /usr/share/raidem/data/spirtle.zip W: raidem uncompressed-zip /usr/share/raidem/data/zippy.zip W: raidem uncompressed-zip /usr/share/raidem/data/bolt.zip W: raidem uncompressed-zip /usr/share/raidem/data/schemer.zip W: raidem uncompressed-zip /usr/share/raidem/data/proj-pulselaser.zip These can also be ignored. These files contain game data that are extracted by the game engine at runtime. MUST ==== * Package and spec named appropriately * License 'zlib' ok, license file included * spec file legible and in Am. English * Builds and runs on FC-5 i386 * No excessive or inappropriate BR: or Requires: * No locales * No shared libraries * Not relocatable * Owns all directories that it creates * No duplicates %files * Permissions look ok * No -devel package necessary * Contains both code and permissible content (game data) * Macro use is consistent and sane * buildroot cleaned during %install and %clean * No .la archives or static libs * desktop file included, looks ok NON-BLOCKING ============ * Source does not match upstream, for a good reason. Upstream source contains a mp3 encoder that can't be included in FE. The game still functions without it, so the offending code has been removed. MUSTFIX ======= * Delete the duplicate copies of the various libs during %prep, not by modifying the upstream tarball. Created attachment 126154 [details]
New specfile which handles the upstream source as requested
Thanks for the review, I've attached a new spec which leaves the upstream
source as intact as possible as requested.
mustfix items fixed. APPROVED imported and build, as usual many thanks for the review. |