Bug 482827 - Review Request: banshee-mirage - An Automatic Playlist Generation Extension for Banshee
Review Request: banshee-mirage - An Automatic Playlist Generation Extension ...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-28 09:03 EST by David Nielsen
Modified: 2009-01-30 07:30 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-30 07:30:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jochen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description David Nielsen 2009-01-28 09:03:00 EST
Spec URL: http://dnielsen.fedorapeople.org/banshee-extension-mirage.spec
SRPM URL: http://dnielsen.fedorapeople.org/banshee-extension-mirage-0.4.0-1.fc10.src.rpm
Description: Mirage is a Banshee plugin that provides innovative spectrum analysis capabilities for the purpose of generating "intelligent" playlists. See also http://hop.at/mirage/ for demonstration videos and further information.
Comment 1 Jochen Schmitt 2009-01-28 14:07:43 EST
Good:
+ Base name of the SPEC file matches to package name
+ Buildroot will be cleaned at the beginn of %clean and %install
+ Local build works fine
+ Can download upstream tar ball with spectool
+ Tar ball in package matches with upstream
(md5sum: 7ebb939d44b64667eaa90ab1ef9a2220)
+ Package contains License tag
+ Package contains a verbatin copy of the license text
+ Buildroot declaration is ok
+ Package contains no patches
+ Package has not subpackages
+ $RPM_OPT_FLAGS are used during the build
+ Excluding of pp64 is ok
+ Local build works fine
+ Start of banshee with install package works
+ Local install works fine
+ Local uninstall works fine
+ Koji build works fine
+ %doc stanza is small, so we need no deparate doc subpackage
+ Packaged files of proper permissions
+ Packaged files are owned by the package
+ Package files have no comflict to other packages
+ Buildroot will be cleaned at the beginning of %clean and %install
+ *.a and *.la file are remove in the %install stanza
+ Package use %find_lang for i18n
+ Proper Changelog


Bad:
- IMHO the package name should be banshee-mirage instead of banshee-extension-mirage
- Mixed use of $RRP_ROOT_BUILD and %{buildroot}
- Package has no %{?_smp_mflags} without a comment why
- Package banshee-musicbrainz-devel doesn't exit in Fedora repository, but
I could not found a blocker bug for a review of this package
- Rpmlint complaints for source package:
$ rpmlint banshee-extension-mirage-0.4.0-1.fc10.src.rpm
banshee-extension-mirage.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 3)
banshee-extension-mirage.src: W: invalid-license X11/MIT
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
- rpmlint complaints binary rpm:
$ rpmlint banshee-extension-mirage-0.4.0-1.fc10.x86_64.rpm
banshee-extension-mirage.x86_64: W: invalid-license X11/MIT
banshee-extension-mirage.x86_64: W: unstripped-binary-or-object /usr/lib64/libmirageaudio.so
banshee-extension-mirage.x86_64: E: invalid-soname /usr/lib64/libmirageaudio.so libmirageaudio.so
1 packages and 0 specfiles checked; 1 errors, 2 warnings.
- License tag catins wrong license specification.
A short review of the copyright notes in the source files show that GPLv2+ may be
the right specificiation for the license tag)
Comment 2 David Nielsen 2009-01-28 15:11:13 EST
banshee-extensions-mirage is consistent both with the application and with other distros (e.g. Ubuntu calls the same package by this name), upholding this as an unofficial standard helps users.

Aside that I think I got all of them, except the unstripped .so file, by all rights now that I removed the nil statement for debug it should be stripped and split into -debug but it isn't.. advice?:

Spec URL: http://dnielsen.fedorapeople.org/banshee-extension-mirage.spec
SRPM URL:
http://dnielsen.fedorapeople.org/banshee-extension-mirage-0.4.0-2.fc10.src.rpm
Comment 3 Jochen Schmitt 2009-01-28 15:39:43 EST
Good:
+ License tag has the right specification GPLv2

Bad:
- Package name don't fits nameing guildlines.
The package is a addon for banshee with the own name of mirage from the upstream author. So the pacnage name should be banshee-mirage. Please refer to
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28General.29
We are not on Debin/Ubuntu. I know, that they have any packages with other package names then here on Fedora.
- Mixed use of $RPM_BUILD_ROOT and %{buildroot}
- Removing of the nil statement was a bad idea, because we get now a debuginfo package which is useless, because the most part of the application is writeen for the mono plattform.
Comment 4 Jochen Schmitt 2009-01-28 15:52:10 EST
You may remove the uncessaries symbols with the strip command
Comment 5 David Nielsen 2009-01-28 16:09:05 EST
yes but that will not put them into the debug package. Without a way to provide debug symbols we can't provide upstream with good backtraces which would make them hate Fedora.. also something about making baby Darwin cry.
Comment 6 David Nielsen 2009-01-28 16:15:49 EST
While I believe the rename is nonsensical and does not serve our users.

Spec URL: http://dnielsen.fedorapeople.org/banshee-mirage.spec
SRPM URL:
http://dnielsen.fedorapeople.org/banshee-mirage-0.4.0-3.fc10.src.rpm
Comment 7 Jochen Schmitt 2009-01-28 16:32:20 EST
Good:
+ Panckage name fits packaging guidelines
+ Consistent usage of rpm macros
+ Package contains parallel build
+ Koji build works fine

Bad:
- Please strip files with thw strip command
- so filesname issue still occurs (No blocker for me)
Comment 8 David Nielsen 2009-01-28 16:47:34 EST
Okay I think this is correct

Spec URL: http://dnielsen.fedorapeople.org/banshee-mirage.spec
SRPM URL:
http://dnielsen.fedorapeople.org/banshee-mirage-0.4.0-4.fc10.src.rpm
Comment 9 Jochen Schmitt 2009-01-29 10:48:27 EST
OK, It's look nice for me, The package is APPROVED.
Comment 10 David Nielsen 2009-01-29 12:15:03 EST
Thank you for the review

New Package CVS Request
=======================
Package Name: banshee-mirage
Short Description: An Automatic Playlist Generation Extension for Banshee
Owners: dnielsen
Branches: F9, F10, devel
InitialCC:
Comment 11 Kevin Fenzi 2009-01-30 01:21:28 EST
cvs done.
Comment 12 Itamar Reis Peixoto 2009-01-30 07:05:48 EST
> Branches: F9, F10, devel

the devel branch are always created, so you don't need to specify it.

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