Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=801092 Martin Erik Werner <martinerikwerner@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martinerikwerner@xxxxxxxxx --- Comment #4 from Martin Erik Werner <martinerikwerner@xxxxxxxxx> 2012-03-12 19:23:38 EDT --- I was encouraged to do an informal review of this package (I am neither reviewer nor sponsor). Apart from things mentioned above, I have: CMAKE OPTIONS: For the bundled enet and tinyxml, it seems like there are cmake options: SUMWARS_NO_TINYXML SUMWARS_NO_ENET CMAKE_BUILD_TYPE:STRING="Release" might also be woth looking at (it's used in packaging/debian/rules) DESKTOP FILE: Icon: should be a basename, not a path, i.e. just "sumwars.png". Exec: -likewise Why are you not using the file available in packaging/sumwars.desktop? (would need to be modded with above fixes though). According to the review guidelines at least, the desktop file name should be sumwars and not fedora-sumwars, so I'm not sure if the --vendor option should be used when installing there... COPYRIGHT: These files seems to be part of ogre: ./tools/meshtest/ExampleApplication.h ./tools/meshtest/ExampleFrameListener.h ./tools/graphicengine/ExampleApplication.h ./tools/graphicengine/ExampleFrameListener.h And are under the custom license: "You may use this sample code for anything you like, it is not covered by the LGPL like the rest of the engine." Are these part of the source that builds Sumwars, or are they only used for the development tools? I guess they could be argued to be under GPLv3 as well, if they are incorporated by Sumwars, otherwise this license needs to be listed, I presume. ./share/resources/gui/imagesets/TaharezLook.tga is both MIT and CC-BY-SA according to ./share/resources/gui/imagesets/License.txt (maybe source was MIT, sumwars derivative is CC-BY-SA?) ./CMakeModules/FindCEGUIOGRE.cmake ./CMakeModules/FindOpenAL.cmake ./CMakeModules/FindCEGUI.cmake Are under the "BSD license" ./CMakeModules/FindLua51.cmake Is GPLv2+ ./src/core/nlfg.h ./src/core/nlfg.cpp Are MIT DEPENDENCY?: Is there a particular why there's a mutual dependency between game <-> data, I don't see why sumwars-data should depend on sumwars? (I'm not an rpm dependency expert, I may be wrong). Just include the %doc in both packages? STRAY FILES: What are these files doing here?: /usr/share/sumwars/resources/packs/OgreCore.zip (also includes 3 font files) /usr/share/sumwars/resources/gui/fonts/DejaVuSans.ttf /usr/share/sumwars/resources/gui/fonts/DejaVuSerif.ttf CHECK?: It might be nice to have a %check section where errorchecker.py is run agains the sumwars binary? That's all I've got so far :) I must say the spec file looks really nice, and I'm definitely going to steal a few things from there to my own in-review package Red Eclipse :D -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review