[Bug 517466] Review Request: lbreakout2 - A breakout-style arcade game for Linux

[bugzilla at redhat.com]

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=517466





--- Comment #6 from Stjepan Gros <stjepan.gros at gmail.com>  2009-08-18 16:32:07 EDT ---
(In reply to comment #4)
> Giving your spec file a once over I see this:
> Use %global instead of %define, unless you really need only locally defined
> submacros within other macro definitions (a very rare case)
> See:
> https://fedoraproject.org/wiki/PackagingGuidelines#.25global_preferred_over_.25define  

Changed.

(In reply to comment #5)
> Some people put the comments to the apply phase, i.e.
> 
> # Enable bar support
> %patch0 -p1
> 
> but I usually put them just before listing them, e.g.
> 
> # This patch enables bar support
> Patch0: foo-bar.patch
> 
> and so on, since normally one looks first at the Patch(N) lines.

I've added comments to PatchN lines.

> What bugs me is that you mix lbreakout2 and %{name}, both of which mean the
> same thing. I'd replace all occurrences of %{name} in %files with lbreakout2.

Replaced.

> Yes, exactly: remove the documentation installed by 'make install' at the end
> of %install with 'rm -rf' and just list the necessary files in %doc, which will
> install them in the correct directory. The current listing

Removed and added to %doc the whole subdirectory with manual files, i.e. docs/.

> PS. You can probably drop the
>  --with-docdir=/usr/share/doc
> argument from %configure. (I haven't looked at the build at all, though.)  

The default location to install the documentation is /usr/lbreakout2/docs so I
used this switch to change the location. Now I removed it since it is not
necessary any more.

New files:

Spec URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/lbreakout2/lbreakout2.spec
SRPM URL:
http://www.zemris.fer.hr/~sgros/stuff/fedora/lbreakout2/lbreakout2-2.6-0.4.beta7.fc11.src.rpm

-- 
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.