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=454867 Nils Philippsen <nphilipp@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |brickshooter - A small |kcirbshooter - A small |puzzle game |puzzle game Flag| |needinfo?(cheekyboinc@fores | |ightlinux.org) --- Comment #10 from Nils Philippsen <nphilipp@xxxxxxxxxx> 2008-10-27 11:49:02 EDT --- 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@xxxxxxx 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. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review