Bad:

MUSTFIX: name probably violates trademark, see http://www.brickshooter.com --
while at it, verify that the contained artwork is indeed made by the author or
properly licensed ("This game was written from scratch in 4 days", I don't know
if this just covers the code or artwork as well).

MUSTFIX: corrected grammar of description:

brickshooter is a small puzzle game for linux, where you have
to clear the central area from differently colored bricks. Three or
more adjacent bricks of the same color will vanish. You can shoot
bricks into the playing field from the fringes.

MUSTFIX: doesn't own all directories that it creates
(/usr/share/icons/hicolor/*/apps) -> Requires: hicolor-icon-theme

MUSTFIX: use $RPM_OPTFLAGS or %optflags as compiler flags (probably needs
changes in the Makefile)

Not so good:

SHOULDFIX: license text not in %doc due to not being in the source tarball --
please ask upstream to include the license, then include it as well.

Good:

- rpmlint checks return:

  1 packages and 0 specfiles checked; 0 errors, 0 warnings.


- package meets naming guidelines (except trademark issue above)
- package meets packaging guidelines
- license (GPLv2) OK
- spec file legible
- source matches upstream
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- package compiles on devel (x86_64)
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime

Additional notes:
- Perhaps "ArcadeGame" to the desktop/menu category?

I have written an email to upstream because of the trademark issue.
Awaiting the response.

Any update from upstream ?

I have written more then one email to upstream.
No response yet.
I will write one more....

I think, neither Nils nor me are absolutely familiar with the US-american 
trademark (see comment #1 and read the "probably"). I don't know how the
whole thing behaves for the US, but as far as I remember, in Germany only
(R) is a registered trademark and TM only marks as trademark but not with
the same permissions as (R), even lower. I maybe misunderstand the english
Wikipedia article, but I think, it tells the same and also I never trust
Wikipedia... ;-)

Anyway, I am not a lawyer, thus I'm adding Tom "spot" Callaway hereby. And
maybe somebody wants to block FE_LEGAL as well, I'm not allowed to set this.

Given that this game is a clear copy of the original, this is an obvious case of trademark infringement. 

If you want to maintain this in Fedora, you're going to need to remove the trademark from all user visible locations (you don't need to change file names or function/variable names, for example) and rename the package.

Normally, I'd suggest that you work with upstream to make this change so you don't have to make such extensive modifications locally.

Blocking FE-Legal.

Any word from upstream here? If not, you have the choice to either:

- Fork it for Fedora, removing the trademark from all user visible locations (you don't need to change file names or function/variable names, for example), make a new tarball with the new name, and maintain it in that fashion.

- Drop this package entirely and close out this ticket.

Nothing from upstream.
I choose the first way.
The package will take some days, but i will do it soon.

Fixed most of the problems and would like to become some feedback...

Spec: http://cheekyboinc.spielen-unter-linux.de/kcirbshooter.spec
SRPM: http://cheekyboinc.spielen-unter-linux.de/kcirbshooter-0.04-1.fc10.src.rpm

All this on top of my first review in comment #1.

Bad:
====

- MUSTFIX: Re-add information about the copyright terms and holder to the source file (luckily there's only one ;-), i.e. I found the following in the original source file and leaving it out may technically be a GPL violation:

...
  License: GPLv2

  paxed

  20080502
...

Possibly just leave the existing top level comment and add yours(*) before that to be on the safe side.

(*) e.g. "kcirbshooter is a fork of the game found at http://... which was done to avoid confusion with the original Brickshooter(tm) game" -- Spot, what do you think?

- MUSTFIX: Don't distribute the compiled executable in the source tarball, it might use the pre-built binary otherwise.

- MUSTFIX: Use either $RPM_BUILD_ROOT or %buildroot consistently, not both.

- MUSTFIX: Use RPM optimization flags. I'll attach a patch which lets you set them this way:

make OPTFLAGS="%optflags" ${?_smp_mflags}"

- MUSTFIX: correct grammar of description:

"""
Kcirbshooter is a small puzzle game for linux, where you have
to clear the central area from differently colored bricks. Three or
more adjacent bricks of the same color will vanish. You can shoot
bricks into the playing field from the fringes.
"""

Good:
=====

- No potential for confusion with original Brickshooter game.
- Owns all directories it creates or depends on packages that contain them.
- Includes license as documentation
- Builds in mock.

(In reply to comment #10)

> (*) e.g. "kcirbshooter is a fork of the game found at http://... which was done
> to avoid confusion with the original Brickshooter(tm) game" -- Spot, what do
> you think?

Sure, that should be within the realm of fair use.

~spot

Created attachment 321634 [details]
Patch which allows setting of OPTFLAGS for the Makefile

Hope all "problems" are fixed now!

Spec: http://cheekyboinc.spielen-unter-linux.de/kcirbshooter.spec
SRPM: http://cheekyboinc.spielen-unter-linux.de/kcirbshooter-0.04-2.fc10.src.rpm

Good:

- Re-added information about the copyright terms and holder to the
source file.
- No compiled executable found in the source tarball.
- Uses $RPM_BUILD_ROOT consistently.
- Uses RPM optimization flags
- Corrected grammar of description.

APPROVED. Go ahead and request CVS to be set up now.

New Package CVS Request
=======================
Package Name: kcirbshooter
Short Description: A small puzzle game
Owners: cheekyboinc
Branches: F-8 F-9 F-10
InitialCC:
Cvsextras Commits: yes

cvs done.