This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 222547 - Review Request: blobby - Blobby Volley 2, a volley-ball game
Review Request: blobby - Blobby Volley 2, a volley-ball game
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michał Bentkowski
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-13 16:43 EST by Aurelien Bompard
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-18 15:38:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Aurelien Bompard 2007-01-13 16:43:36 EST
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/blobby.spec
SRPM URL: http://gauret.free.fr/fichiers/rpms/fedora/blobby-0.6-0.1.a.src.rpm
Description: 
Blobby Volley is one of the most popular freeware games.
Blobby Volley 2 is the continuation of this lovely game.
Comment 1 Michał Bentkowski 2007-01-13 17:45:18 EST
I'll review it.
Comment 2 Michał Bentkowski 2007-01-13 18:24:11 EST
REVIEW:
 * sources match upstream (md5: fdef3e59f0870d890be8dffaccd773ca)
 * package meets all guidelines it should
 * correct build root: %{_tmppath}/%{name}-%{version}-%{release}-
root-%(%{__id_u} -n)
 * package is licensed under a GPL license and license text is included in 
package
 * the latest version is being packaged
 * no shared libraries
 * no duplicates in %files
 * %clean is present
 * proper scriptlets
 * no need to any subpackages
 * rpmlint is silent
 * final provides and requires are sane
!* desktop file:
Category Application is deprecated and should be removed.
https://www.redhat.com/archives/fedora-extras-list/2006-October/msg00723.html
 * BRs listed good
 * mock builds fine (fc6/x86_64)
 * all directories are owned well
!* a lack of Requires:
you should add hicolor-icon-theme dependency:
https://www.redhat.com/archives/fedora-extras-list/2006-September/msg00282.html

THINGS to do:
 - get rid of an Application category
 - add hicolor-icon-theme dependency
Comment 3 Aurelien Bompard 2007-01-14 06:14:50 EST
Thanks for the review. I've added the hicolor-icon-theme dependency, but I'm not
sure about the Application category, since the example on the Packaging
Guidelines page still has it :
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

I've posted on the packaging list for advice, I'll let you know the results.
Comment 4 Michał Bentkowski 2007-01-14 06:19:46 EST
(In reply to comment #3)
> Thanks for the review. I've added the hicolor-icon-theme dependency, but I'm 
not
> sure about the Application category, since the example on the Packaging
> Guidelines page still has it :
> http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

So that should be fixed. There's desktop-file-utils-0.12 in fc7 and its
desktop-file-validate shows the following error: 

[ecik@ecik /download]$ desktop-file-validate /usr/share/applications/fedora-
blobby.desktop
/usr/share/applications/fedora-blobby.desktop: warning: The 'Application' 
category is not defined by the desktop entry specification.  Please use one of 
"AudioVideo", "Audio", "Video", "Development", "Education", "Game", "Graphics", 
"Network", "Office", "Settings", "System", "Utility" instead

Comment 5 Aurelien Bompard 2007-01-14 07:43:57 EST
OK then, thanks.

* Sun Jan 14 2007 Aurelien Bompard <abompard@fedoraproject.org> 0.6-0.2.a
- add dependency on hicolor-icon-theme (#222547)
- removed the "Application" category from the desktop file

http://gauret.free.fr/fichiers/rpms/fedora/blobby.spec
http://gauret.free.fr/fichiers/rpms/fedora/blobby-0.6-0.2.a.src.rpm
Comment 6 Michał Bentkowski 2007-01-14 12:21:30 EST
Approved.
Comment 7 Aurelien Bompard 2007-01-14 12:45:48 EST
Just something I caught at the last minute:

* Sun Jan 14 2007 Aurelien Bompard <abompard@fedoraproject.org> 0.6-0.3.a
- patch it to obey RPM_OPT_FLAGS

http://gauret.free.fr/fichiers/rpms/fedora/blobby.spec
http://gauret.free.fr/fichiers/rpms/fedora/blobby-0.6-0.3.a.src.rpm

Please tell if you still approve it with this change.
Comment 8 Mamoru TASAKA 2007-01-14 13:06:47 EST
Please don't use autotool when possible.

It seems that the flag "-Os" is used only at one point
in configure, so making a patch for configure, not
for configure.in should be easy.
Comment 9 Aurelien Bompard 2007-01-14 13:47:59 EST
Sure, I don't mind, but why ? What's the problem with it ?

Anyway, I've patched configure instead :
http://gauret.free.fr/fichiers/rpms/fedora/blobby.spec
http://gauret.free.fr/fichiers/rpms/fedora/blobby-0.6-0.4.a.src.rpm
Comment 10 Mamoru TASAKA 2007-01-14 14:11:07 EST
(In reply to comment #9)
> Sure, I don't mind, but why ? What's the problem with it ?

Well, 
* A weak reason is just "please don't use which is not needed
  actually".
* Another reason is that using autotool easily may generate
  files which upstream (or someone else) may not desire,
  especially when the version of autotool differ between
  which upstream used and which you used.

  This is once discussed when autotools are removed from
  minimum buildroot, starting from
http://www.redhat.com/archives/fedora-maintainers/2006-September/msg00146.html
Comment 11 Aurelien Bompard 2007-01-14 14:41:09 EST
Fine, thanks for this clear reply.
Comment 12 Michał Bentkowski 2007-01-14 15:26:13 EST
Package is still approved, of course.
Comment 13 Aurelien Bompard 2007-01-18 15:38:29 EST
Imported and built, thanks !

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