Bug 730851

Summary: Review Request: chocolate-doom - Conservative Doom source port
Product: [Fedora] Fedora Reporter: Rahul Sundaram <metherid>
Component: Package ReviewAssignee: Ankur Sinha (FranciscoD) <sanjay.ankur>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: notting, package-review, sanjay.ankur
Target Milestone: ---Flags: sanjay.ankur: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-08-24 07:31:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Rahul Sundaram 2011-08-15 23:18:08 UTC
Spec URL: http://sundaram.fedorapeople.org/packages/chocolate-doom.spec
SRPM URL: http://sundaram.fedorapeople.org/packages/chocolate-doom-1.6.0-1.src.rpm
Description: 
Chocolate Doom aims to accurately reproduce the experience of playing vanilla 
Doom. It is a conservative, historically accurate Doom source port, which is 
compatible with the thousands of mods and levels that were made before the 
Doom source code was released. Rather than flashy new graphics, Chocolate Doom's
main features are its accurate reproduction of the game as it was played in the 
1990s.

Comment 1 Ankur Sinha (FranciscoD) 2011-08-17 18:39:02 UTC
Hello,

I shall take up this ticket. 

Thanks,
Ankur

Comment 2 Ankur Sinha (FranciscoD) 2011-08-20 10:34:22 UTC
Review:

+ OK
? ISSUE
- NA

+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:

- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

+ Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.
^^ 
Only incorrect-fsf-addresses. Please contact upstream

+ final provides and requires are sane:
== chocolate-doom-1.6.0-1.i686.rpm ==
Provides:
chocolate-doom = 1.6.0-1
chocolate-doom(x86-32) = 1.6.0-1

Requires:
libSDL-1.2.so.0  
libSDL_mixer-1.2.so.0  
libSDL_net-1.2.so.0  
libc.so.6  
libc.so.6(GLIBC_2.0)  
libc.so.6(GLIBC_2.1)  
libc.so.6(GLIBC_2.3)  
libc.so.6(GLIBC_2.3.4)  
libc.so.6(GLIBC_2.4)  
libm.so.6  
libm.so.6(GLIBC_2.0)  
libpthread.so.0  
libpthread.so.0(GLIBC_2.0)  
libsamplerate.so.0  
libsamplerate.so.0(libsamplerate.so.0.0)  
rtld(GNU_HASH)  

== chocolate-doom-1.6.0-1.src.rpm ==
Provides:

Requires:
SDL-devel  
SDL_mixer-devel  
SDL_net-devel  
libsamplerate-devel  
desktop-file-utils  

== chocolate-doom-debuginfo-1.6.0-1.i686.rpm ==
Provides:
chocolate-doom-debuginfo = 1.6.0-1
chocolate-doom-debuginfo(x86-32) = 1.6.0-1

Requires:


SHOULD Items:

+ Should build in mock.
+ Should build on all supported archs
- Should function as described.
+ Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
? Should have dist tag
^^ 
Please consider using the dist tag

+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:
1. The package uses an md5 implementation. 
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Packages_granted_exceptions

Please add a provides for it as mentioned on the page.

2. Please consider using the dist tag

3. Please mail upstream requesting them to update the FSF address.

The rest looks good :)


Thanks,
Ankur

Comment 3 Ankur Sinha (FranciscoD) 2011-08-20 10:38:36 UTC
md5sum:

[ankur@ankur dump]$ md5sum chocolate-doom-1.6.0.tar.gz ~/rpmbuild/SOURCES/chocolate-doom-1.6.0.tar.gz 
b210e115dda6ea88bfb6c4fe11ade101  chocolate-doom-1.6.0.tar.gz
b210e115dda6ea88bfb6c4fe11ade101  /home/ankur/rpmbuild/SOURCES/chocolate-doom-1.6.0.tar.gz

Comment 4 Rahul Sundaram 2011-08-20 10:58:48 UTC
Mailed upstream for FSF address update.  Fixed other two issues

http://sundaram.fedorapeople.org/packages/chocolate-doom.spec
http://sundaram.fedorapeople.org/packages/chocolate-doom-1.6.0-2.fc15.src.rpm

Comment 5 Ankur Sinha (FranciscoD) 2011-08-20 11:01:42 UTC
Alright!

Issues have been fixed! XX APPROVED XX 

Thanks,
Ankur

Comment 6 Rahul Sundaram 2011-08-20 11:37:17 UTC
New Package SCM Request
=======================
Package Name: chocolate-doom
Short Description: Historically compatible Doom engine
Owners: sundaram
Branches: f16
InitialCC:

Comment 7 Gwyn Ciesla 2011-08-20 15:19:52 UTC
Git done (by process-git-requests).