Bug 503519 - Review Request: bastet - An evil falling bricks game
Review Request: bastet - An evil falling bricks game
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
:
: 509673 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-01 10:50 EDT by Stefan Posdzich
Modified: 2009-07-04 19:54 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-08 15:32:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tcallawa: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Stefan Posdzich 2009-06-01 10:50:07 EDT
Spec URL:

http://cheekyboinc.fedorapeople.org/bastet.spec

SRPM URL:

http://cheekyboinc.fedorapeople.org/bastet-0.43-1.fc11.src.rpm

Description:

Bastet stands for "bastard tetris", and is a simple ncurses-based 
Tetris(R) clone for Linux. Unlike normal Tetris(R), however, Bastet 
does not choose your next brick at random. Instead, it uses a special 
algorithm designed to choose the worst brick possible. As you can 
imagine, playing Bastet can be a very frustrating experience!

rpmlint: no output
scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1387035
md5sum: 

local b47090daa7b6d89b98b5b477cf155733  bastet-0.43.tgz
upstream b47090daa7b6d89b98b5b477cf155733  bastet-0.43.tgz
Comment 1 Tom "spot" Callaway 2009-06-01 12:11:17 EDT
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.
Comment 2 Stefan Posdzich 2009-06-01 17:35:08 EDT
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
Comment 3 Tom "spot" Callaway 2009-06-03 14:55:48 EDT
You missed the manpage... but that looks like the only item that you missed. Clean it up and I'll lift FE-Legal.
Comment 4 Stefan Posdzich 2009-06-04 06:29:43 EDT
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
Comment 5 Tom "spot" Callaway 2009-06-04 09:10:26 EDT
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.
Comment 6 Stefan Posdzich 2009-06-05 10:11:56 EDT
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 =)
Comment 7 Tom "spot" Callaway 2009-06-05 10:44:42 EDT
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. ;)
Comment 8 Stefan Posdzich 2009-06-05 13:18:52 EDT
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? :)
Comment 9 Tom "spot" Callaway 2009-06-05 15:01:48 EDT
Look at the gcc invocations. ;)
Comment 10 Stefan Posdzich 2009-06-06 06:47:56 EDT
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
Comment 11 Tom "spot" Callaway 2009-06-06 09:46:54 EDT
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.
Comment 12 Stefan Posdzich 2009-06-06 10:55:55 EDT
New Package CVS Request
=======================
Package Name: bastet
Short Description: An evil falling bricks game
Owners: cheekyboinc
Branches: F-10 F-11
Comment 13 Jason Tibbitts 2009-06-08 12:19:49 EDT
CVS done.
Comment 14 Fedora Update System 2009-06-08 15:29:55 EDT
bastet-0.43-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/bastet-0.43-5.fc10
Comment 15 Fedora Update System 2009-06-15 22:00:34 EDT
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.
Comment 16 Susi Lehtola 2009-07-04 19:54:26 EDT
*** Bug 509673 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.