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.
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)
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
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.
You may remove the uncessaries symbols with the strip command
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.
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
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)
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
OK, It's look nice for me, The package is APPROVED.
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:
cvs done.
> Branches: F9, F10, devel the devel branch are always created, so you don't need to specify it.
http://koji.fedoraproject.org/koji/buildinfo?buildID=80907