Bug 564425 - Review Request: sugar-tamtam - A suite of four music and sound related activities
Review Request: sugar-tamtam - A suite of four music and sound related activi...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On:
Blocks: SOAS-3
  Show dependency treegraph
 
Reported: 2010-02-12 12:31 EST by Sebastian Dziallas
Modified: 2010-02-28 20:38 EST (History)
3 users (show)

See Also:
Fixed In Version: sugar-tamtam-0-0.3.20100201git.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-02-28 20:38:07 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Sebastian Dziallas 2010-02-12 12:31:29 EST
Spec URL: http://sdz.fedorapeople.org/rpmbuild/sugar-tamtam.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuild/sugar-tamtam-0-0.1.20100201git.fc12.src.rpm

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1980419
Description: This is another activity from the Sugar packaging effort.
Comment 1 Christoph Wickert 2010-02-24 20:53:06 EST
FIX - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/sugar-tamtam-*
sugar-tamtam.src: W: spelling-error %description -l en_US Csound -> Sound, C sound, Compound
sugar-tamtam.src: W: non-standard-group Sugar/Activities
sugar-tamtam.src: W: invalid-url Source0: sugar-tamtam-0-20100201.tar.bz2
sugar-tamtam.x86_64: W: spelling-error %description -l en_US Csound -> Sound, C sound, Compound
sugar-tamtam.x86_64: W: non-standard-group Sugar/Activities
sugar-tamtam.x86_64: E: backup-file-in-package /usr/share/tamtam/Sounds/templebell.orig
sugar-tamtam.x86_64: E: backup-file-in-package /usr/share/tamtam/Sounds/sarangi.orig
sugar-tamtam.x86_64: E: backup-file-in-package /usr/share/tamtam/Sounds/drum6kit.orig
sugar-tamtam-edit.noarch: W: spelling-error %description -l en_US organise -> organist, organism, organize
sugar-tamtam-edit.noarch: W: non-standard-group Sugar/Activities
sugar-tamtam-edit.noarch: W: dangling-symlink /usr/share/sugar/activities/TamTamEdit.activity/common /usr/share/tamtam
sugar-tamtam-jam.noarch: W: spelling-error %description -l en_US polyphonically -> poly phonically, poly-phonically, polyphonic ally
sugar-tamtam-jam.noarch: W: spelling-error %description -l en_US beatbox -> beat box, beat-box, Beatrix
sugar-tamtam-jam.noarch: W: spelling-error %description -l en_US rythms -> rhythms, rhythm, algorithms
sugar-tamtam-jam.noarch: W: non-standard-group Sugar/Activities
sugar-tamtam-jam.noarch: W: dangling-symlink /usr/share/sugar/activities/TamTamJam.activity/common /usr/share/tamtam
sugar-tamtam-mini.noarch: W: non-standard-group Sugar/Activities
sugar-tamtam-mini.noarch: W: dangling-symlink /usr/share/sugar/activities/TamTamMini.activity/common /usr/share/tamtam
sugar-tamtam-synthlab.noarch: W: spelling-error %description -l en_US synthesiser -> synthesizer, synthesis er, synthesis-er
sugar-tamtam-synthlab.noarch: W: spelling-error %description -l en_US modelled -> modeled, model led, model-led
sugar-tamtam-synthlab.noarch: W: non-standard-group Sugar/Activities
sugar-tamtam-synthlab.noarch: W: dangling-symlink /usr/share/sugar/activities/TamTamSynthLab.activity/common /usr/share/tamtam
7 packages and 0 specfiles checked; 3 errors, 19 warnings.

The following warnings/errors can be ignored: non-standard-group, spelling-error for csound, dangling-symlink (due to the subpackages). This leaves two typos and backup-file-in-package. Are the *.orig files needed?

OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines (GPLv2+ and GPLv3+)
OK - MUST: License field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
N/A - MUST: sources match the upstream source by MD5 (git snapshot)
OK - MUST: successfully compiles and builds into binary rpms on x86_64 (noarch)
N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
OK - MUST: all build dependencies are listed in BuildRequires.
OK - MUST: handles locales properly with %find_lang
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
OK - MUST: Package does not bundle copies of system libraries.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review.
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
OK - MUST: consistently uses macros
OK - MUST: package contains code, or permissable content
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package.
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: package does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock.
OK - SHOULD: compiles and builds into binary rpms on all supported architectures.
OK - SHOULD: functions as described.
N/A - SHOULD: Scriptlets are used, those scriptlets must be sane.
OK - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - Compiler flags ok
OK - Debuginfo complete


Issues:
- The sugar-tamtam package should be called sugar-tamtam-common because it is not a standalone package. The srpm and spec will still be called sugar-tamtam.

- Due to python3 the python guidelines have changed recently. Please update the macro definitions acording to https://fedoraproject.org/wiki/Packaging/Python#Macros

- You can drop python_sitelib because it is not needed here.

- make is not verbose so compiler flags cannot be verified.

- add -p to preserve timestamps when using cp -r 

- the command to install the package should be
%{__python} setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT
instead of
python ./setup.py install --prefix=$RPM_BUILD_ROOT/%{_prefix}

- Typos: organise -> organize, synthesiser -> synthesizer

- backup-file-in-package: Are these files needed?
Comment 2 Sebastian Dziallas 2010-02-27 17:07:17 EST
(In reply to comment #1)

I uploaded a new version.

Spec URL: http://sdz.fedorapeople.org/rpmbuild/sugar-tamtam.spec
SRPM URL: http://sdz.fedorapeople.org/rpmbuild/sugar-tamtam-0-0.2.20100201git.fc12.src.rpm

> Issues:
> - The sugar-tamtam package should be called sugar-tamtam-common because it is
> not a standalone package. The srpm and spec will still be called sugar-tamtam.

I moved the common files into a common package. Rpmlint complains now about the main package containing no binary, though.

> - Due to python3 the python guidelines have changed recently. Please update the
> macro definitions acording to
> https://fedoraproject.org/wiki/Packaging/Python#Macros
> 
> - You can drop python_sitelib because it is not needed here.

Both done!

> - make is not verbose so compiler flags cannot be verified.

I added the V=1 parameter, so that should be good, too.

> - add -p to preserve timestamps when using cp -r 

Done.

> - the command to install the package should be
> %{__python} setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT
> instead of
> python ./setup.py install --prefix=$RPM_BUILD_ROOT/%{_prefix}

We discussed this on IRC; setup.py seems to have issues with the other invocation.
 
> - Typos: organise -> organize, synthesiser -> synthesizer

Fixed these and a few other typos.

> - backup-file-in-package: Are these files needed?    

I don't think so and have removed them.
Comment 3 Christoph Wickert 2010-02-27 17:41:22 EST
(In reply to comment #2)
> I moved the common files into a common package. Rpmlint complains now about the
> main package containing no binary, though.

This is ok, the lib in %{python_sitearch}/tamtam *is* binary.

APPROVED
Comment 4 Sebastian Dziallas 2010-02-27 18:03:45 EST
New Package CVS Request
=======================
Package Name: sugar-tamtam
Short Description: A suite of four music and sound related activities
Owners: sdz
Branches: F-13 F-12 F-11
Comment 5 Sebastian Dziallas 2010-02-27 18:04:26 EST
Thanks for the review! I'll import an updated version that doesn't build the - empty - base package, as discussed on IRC.
Comment 6 Jason Tibbitts 2010-02-28 17:32:32 EST
CVS done (by process-cvs-requests.py).
Comment 7 Fedora Update System 2010-02-28 18:04:16 EST
sugar-tamtam-0-0.3.20100201git.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/sugar-tamtam-0-0.3.20100201git.fc13
Comment 8 Fedora Update System 2010-02-28 20:38:02 EST
sugar-tamtam-0-0.3.20100201git.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

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