Bug 469492

Summary: Review Request: bam - A fast and flexible build system
Product: [Fedora] Fedora Reporter: Simon <cassmodiah>
Component: Package ReviewAssignee: Lubomir Rintel <lkundrak>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, lkundrak, notting
Target Milestone: ---Flags: lkundrak: fedora-review+
kevin: 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: 2008-11-10 05:29:13 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:
Bug Depends On:    
Bug Blocks: 462181    

Comment 1 Lubomir Rintel 2008-11-02 17:54:02 UTC
The package seems pretty clean and well done -- thanks! However, there are some (mostly cosmetic) issues:

1.) Fix the version number

Version:        0.0.0.for.teeworlds.0.4.3
# bam is not versioned

Please don't make up version numbers like this. Using "0.4.3" as a version number would perfectly do.

2.) Use the original source code from upstream.

Source0:        http://cassmodiah.fedorapeople.org/%{name}-%{version}/SOURCES/%{name}-%{version}.tar.bz2

If you extracted this from a tarball with a public URL, use that one. If you checked it from a SCM, add a comment (and adjust the Release tag to the date you checked it out).

3.) Please improve Summary and description a bit:

Summary:        A fast and flexible build system

Package Guidelines read: "Please put personal preferences aside..." while "fast and flexible" seem much like a personal preference.

%description
the bam build system for teeworlds 

Please use capitalization and punctuation correctly. Please consider enhancing the description a bit, so even a non-tech user understand what does the package do (and that he doesn't want to install it :). "A tool that controls process of producing executables of software from its source code. Used to build Teeworlds game." or something similar might be appropriate.

4.) Don't install bam into sbin

install -D -m 0755 src/%{name} \
        %{buildroot}%{_sbindir}/%{name}

According to FHS sbin is intended for system administration tools, mainly used by root user. BAM is not used for administration and definitely shouldn't be run by root.

5.) Possibly do not include TODO

sh todo.sh > TODO

The produced file doesn't really make sense to anyone.

Comment 2 Lubomir Rintel 2008-11-02 18:20:33 UTC
Builds fine in mock (tried in el5).
RPMlint is silent and happy.

And there's one more issue:

6.) Please honour optflags

sh make_unix.sh

This should better be replaced with

sh -x make_unix.sh %{optflags}

(The -x is there so that the log from build output is more legible)

Note that the optflags aren't used also when you build "txt2c". Since it is not installed, but used only during the build process, I don't believe it's an issue. If you want to build it with optflags, add "$@" to the corresponding line of make_unix.sh.

Comment 3 Simon 2008-11-04 20:43:41 UTC
SPEC:
http://cassmodiah.fedorapeople.org/bam-0.0.0.4.3/bam.spec

SRPM:
http://cassmodiah.fedorapeople.org/bam-0.0.0.4.3/bam-0.0.0.4.3-1.fc10.src.rpm


---------
rpmlint  /var/lib/mock//epel-5-i386/result/bam-*
bam.src: W: uncompressed-zip /bam.zip
3 packages and 0 specfiles checked; 0 errors, 1 warnings.
---------

Comment 4 Lubomir Rintel 2008-11-04 21:20:47 UTC
Looks fine this time. Thanks!

APPROVED

Comment 5 Simon 2008-11-04 21:40:04 UTC
thank you very much!


New Package CVS Request
=======================
Package Name: bam 
Short Description: A build system
Owners: cassmodiah lkundrak 
Branches: F-9 F-10 EL-5
InitialCC:

Comment 6 Kevin Fenzi 2008-11-05 22:38:00 UTC
The description seems very generic here. Let me know in a new request if you would like to update it to something more descriptive. 

cvs done.

Comment 7 Fedora Update System 2008-11-08 14:30:07 UTC
bam-0.0.0.4.3-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/bam-0.0.0.4.3-1.fc10

Comment 8 Fedora Update System 2008-11-22 16:50:32 UTC
bam-0.0.0.4.3-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.