Bug 435514 - Review Request: lbrickbuster2 - Brickbuster arcade game
Summary: Review Request: lbrickbuster2 - Brickbuster arcade game
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Conrad Meyer
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 517466 629468 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-02-29 20:39 UTC by Hans de Goede
Modified: 2010-09-02 15:25 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-03-04 17:50:29 UTC
Type: ---
Embargoed:
cse.cem+redhatbugz: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Hans de Goede 2008-02-29 20:39:26 UTC
Spec URL: http://people.atrpms.net/~hdegoede/lbrickbuster2.spec
SRPM URL: http://people.atrpms.net/~hdegoede/lbrickbuster2-2.6-0.9.beta7.fc9.src.rpm
Description: 
The successor to LBrickBuster offers you a new challenge in more than 50 levels
with loads of new bonuses (goldshower, joker, explosive balls, bonus magnet
...), maluses (chaos, darkness, weak balls, malus magnet ...) and special
bricks (growing bricks, explosive bricks, regenerative bricks ...). If you
are still hungry for more after that you can create your own levelsets with
the integrated level editor.

Comment 1 Conrad Meyer 2008-03-02 13:43:48 UTC
+ for good, - for bad, ? for questionable

MUST Items:
? rpmlint:
  lbrickbuster2.x86_64: E: non-standard-executable-perm /usr/bin/lbrickbuster2 
02551
  lbrickbuster2.x86_64: E: non-standard-executable-perm /usr/bin/lbrickbuster2 
02551
+ package named according to naming guidelines
+ spec file name matches base package name
+ packaging guidelines
+ license (GPLv2+)
+ license matches source files (I just checked game.c, assumed the rest are the 
same)
+ COPYING included in RPM
+ spec is in en_US
+ spec is quite legible
+ sources match upstream
+ compiles/builds on x86_64
+ build dependencies listed in BR (tested on koji)
+ locales handled properly
+ if shared libraries, must call ldconfig (don't have any)
+ no Prefix: /usr
+ don't hostile takeover other packages' directories
+ no duplicate %files
? proper permissions on files (see rpmlint output above)
+ %clean section
- consistent use of macros (you mix $VARIABLES and %{variables})
+ package contains code and permissible content
+ no need for -doc
+ %doc doesn't affect runtime
+ no header files
+ no static libraries
+ no pkgconfig
+ no library files with suffixes
+ no devel package
+ no .la libtool archives
+ %{name}.desktop installed
+ package doesn't attempt to own other packages' files
+ %install cleans up build root
+ filenames are valid UTF8

SHOULD Items:
+ source includes license texts from upstream
? descriptions/summary don't contain non-English translations, but I doubt they 
are available
+ package builds in mock 
(http://koji.fedoraproject.org/koji/taskinfo?taskID=484020)
+ package compiles into binary RPMs on all archs (see above)
+ package functions as described (it's fun :))
+ no file dependencies

Clean up the macros and assure me that the permissions are OK and I'll set 
fedora-review to +.

Comment 2 Kevin Kofler 2008-03-02 17:48:34 UTC
The executable should probably have 02755 perms rather than 02551.

Comment 3 Hans de Goede 2008-03-02 22:06:31 UTC
(In reply to comment #1)
> + for good, - for bad, ? for questionable
> 
> MUST Items:
> ? rpmlint:
>   lbrickbuster2.x86_64: E: non-standard-executable-perm /usr/bin/lbrickbuster2 
> 02551
>   lbrickbuster2.x86_64: E: non-standard-executable-perm /usr/bin/lbrickbuster2 
> 02551

This is the same permission scheme the gnome-games package uses for sgid games
games, and these are the permissions as used for this package by freshrpms for a
long time, but I'm happy to change them to 02755 if people like that better
(which will still make rpmlint complain btw)

> - consistent use of macros (you mix $VARIABLES and %{variables})

Erm, no I don't atleast not in a way thats not allowed by the guidelines. The
only $ variable I use is $RPM_BUILD_ROOT, and I don't use %{buildroot} anywhere,
so no mixing, the guidelines talk about using 2 different ways to access the
_same_ variable. As for using $RPM_BUILD_ROOT versus %{buildroot}, the
guidelines leave this up to the packager, and AFAIK most people prefer
$RPM_BUILD_ROOT, that is for example which is used in the specfile templates
which are part of rpmdevtools.

> Clean up the macros
I would love to, if you can tell me whats wrong exactly.

> and assure me that the permissions are OK
The permissions are OK.




Comment 4 Kevin Kofler 2008-03-02 22:42:25 UTC
The reason why ??1 permissions are frowned upon is that it doesn't make sense 
to read-protect an executable which can be easily obtained from any Fedora 
mirror. But rpmlint probably whines about any 2??? permission, it doesn't like 
setgid (but setgid games is normal for game packages, so the warning can be 
ignored).

Comment 5 Hans de Goede 2008-03-03 08:43:56 UTC
(In reply to comment #4)
> The reason why ??1 permissions are frowned upon is that it doesn't make sense 
> to read-protect an executable which can be easily obtained from any Fedora 
> mirror. But rpmlint probably whines about any 2??? permission, it doesn't like 
> setgid (but setgid games is normal for game packages, so the warning can be 
> ignored).

Okay, I will fix the permissions before import.

New Package CVS Request
=======================
Package Name:      lbrickbuster2
Short Description: Brickbuster arcade game
Owners:            jwrdegoede
Branches:          F-8
InitialCC: 
Cvsextras Commits: Yes


Comment 6 Kevin Fenzi 2008-03-03 20:11:24 UTC
cvs done.

Comment 7 Hans de Goede 2008-03-04 17:50:29 UTC
Thanks for the review!

Imported and build, closing.


Comment 8 Stjepan Gros 2009-08-28 06:00:38 UTC
*** Bug 517466 has been marked as a duplicate of this bug. ***

Comment 9 Germán Racca 2010-09-02 15:25:53 UTC
*** Bug 629468 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.