Bug 503519
| Summary: | Review Request: bastet - An evil falling bricks game | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Stefan Posdzich <cheekyboinc> |
| Component: | Package Review | Assignee: | Tom "spot" Callaway <tcallawa> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, notting, steve, tcallawa |
| Target Milestone: | --- | Flags: | tcallawa:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-06-08 19:32:15 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: | |||
|
Description
Stefan Posdzich
2009-06-01 14:50:07 UTC
Yeah... so you're going to have to remove the visible references to the "Tetris" trademark here. The rule of thumb is: "If it is visible to the end user, it needs to go away." This includes your spec Summary and Description, any places in the code where "Tetris" is displayed to the user, and any references in the documentation. Once you've done that, please post an updated SRPM and I'll audit it. Spec URL: http://cheekyboinc.fedorapeople.org/bastet.spec SRPM URL: http://cheekyboinc.fedorapeople.org/bastet-0.43-2.fc11.src.rpm - Removed reference to Tetris to match our guidelines You missed the manpage... but that looks like the only item that you missed. Clean it up and I'll lift FE-Legal. Spec URL: http://cheekyboinc.fedorapeople.org/bastet.spec SRPM URL: http://cheekyboinc.fedorapeople.org/bastet-0.43-3.fc11.src.rpm - Add manpage - Removed reference to Tetris in the bastet manpage Trademark stuff looks good now, thanks.
A few other issues that need to be resolved:
* The License: should be GPLv3+ (look at the code headers).
* You're also not building it with %{optflags}. (passing CXXFLAGS="%{optflags}" to make should do the trick)
* You're also using the old icon cache scriptlets, see:
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
* You do not need to manually gzip the manpage, just install it uncompressed into the proper mandir and rpm will gzip it properly for you.
I think that fixing those items should be enough for me to finish the review. Lifting FE-Legal.
Spec URL: http://cheekyboinc.fedorapeople.org/bastet.spec SRPM URL: http://cheekyboinc.fedorapeople.org/bastet-0.43-4.fc11.src.rpm - Added new icon cache scriptlets - Added optflags - Changed license to GPLv3+ - Removed manually gzip of manpage Hope its all fixed now =) Look closely at how you're passing optflags, and how I suggested you do it. Do a build and see if you can see the optflags being used. ;) CFLAGS="%{optflags}"
+ make -j3 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic -fasynchronous-unwind-tables'
vs.
CXXFLAGS="%{optflags}"
+ make -j3 'CXXFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic -fasynchronous-unwind-tables'
So whats the difference? :)
Look at the gcc invocations. ;) Ah yes :) There it is. Spec URL: http://cheekyboinc.fedorapeople.org/bastet.spec SRPM URL: http://cheekyboinc.fedorapeople.org/bastet-0.43-5.fc11.src.rpm - Changed CFLAGS to CXXFLAGS Good: - rpmlint checks return nothing. - package meets naming guidelines - package meets packaging guidelines - license (GPLv3+) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream (1d7d42f93831b903e1fccee106a71c0bd4822c3d) - package compiles on devel (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - desktop file okay, handled properly APPROVED. New Package CVS Request ======================= Package Name: bastet Short Description: An evil falling bricks game Owners: cheekyboinc Branches: F-10 F-11 CVS done. bastet-0.43-5.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/bastet-0.43-5.fc10 bastet-0.43-5.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. *** Bug 509673 has been marked as a duplicate of this bug. *** |