Bug 666633

Hallo,

here some pre-review complaints:

1.) I dont't like the package name liblastfmlib. It may be better to choich lastfmlib as an package name.

2.) you hve the buildRoot tag twice in your package. Current RPM releases doesn't need an BuildRoot tag. They are required only for EPEL-branches.

3.) the devel subpackage need a Requires to the main package in the form: Requires: %{name} = %{version}-%{release}

4.) Instead of '-n liblastfmlib-devel' you shoud prefer to write only 'devel' ot refer to the subpackage in the tags.

5.) It may be nice to insert a blank line between the %package and the %description stanza.

6.) Please us %{_includedir} instead of /usr/include

7.) Please use %{?_smp_mflags} to initiate a parallel build.

8:) Don't use the %makeinstall macro

9.) You don't need to specified the name of the main package in the %file stanza

10.) Please remove the *.la file from the devel package

11.) The license tag have to been GPLv2+. Please examinate the copyrith notes on the top of the sources files. This notes say, that you ca use the second version of the GPL or any later version, so you have to use GPLv2+ for the license tag.

12.) Upstream tar ball contains a verbatin copy of the license text which was not included in the package on the %doc stanza.

13 Pakcage has not %doc stanza with files like README which are provided by upstream authow

14.) You should use %{_includedir}/lastfm/ in the %files stanza to make sure, that this directory is onwed by the package. All Files and directories belong this directory will be included to the package, so no additional specification is required

15.) the specification of gcc-c++ as a BuildRequires is not required

Sorry for the long list of complaints, but if you are able to create a proper
package, I may will sponsor you.

Best Regards:

Jochen Schmitt

I see in your changelog comments that you modified someone else’s specfile, which is probably why there are so many little issues.  You might want to use the command “rpmdev-newspec -t lib lastfmlib” to create a new skeleton specfile that conforms to Fedora guidelines, and work from there (rpmdev-newspec is in the rpmdevtools package).  Most of the things that Jochen addressed are laid out in the skeleton configuration, you'll just need to fill in the blanks.  I also found it very helpful to look at the packages already included in Fedora when I first started out to get familiar with the conventions people used.  Package sources are at http://pkgs.fedoraproject.org/gitweb

Thank you both for your precious advices. 

I uploaded new files and renamed the library to lastfmlib.

Spec URL: http://www.tepsonic.org/files/fedora/lastfmlib.spec
SRPM URL: http://www.tepsonic.org/files/fedora/lastfmlib-0.4.0-1.fc14.src.rpm

Thast is now an official review:

Good:
+ Package name fullfill naming guidelines
+ Package has consistantly rpm macro usage
+ Package contains URL tag to project homepage
+ Package contains recent version of the software
+ License tag contains a valid OSS license
+ License tag refer GPLv2+ as as valid license
+ Package contains verbatin copy of the license text
+ Package has a devel sub package
+ Source tag shows on proper dowload location
+ package tar ball matches with upstram one
(md5sum: 6f00882c15b8cc703718d22e1b1871f)
+ local build works fine
+ Parallel build is supported by the package
+ debuginfo rpm contains source files
+ Build on koji (rawhide) works fine
+ local install und uninstall works fine
+ All file and directories are owned by the package
+ There are not naming conflicts with other packages
+ %doc stanza is small, so we need no separate sub package.
+ Changelog entries has proper format

Please check:
? Rpmlist shows folling warning on source rpm:
lastfmlib.src: W: spelling-error Summary(en_US) scrobbling -> scribbling, scrabbling
lastfmlib.src: W: spelling-error %description -l en_US scrobble -> scribble, scrabble
lastfmlib.src: W: strange-permission lastfmlib-0.4.0.tar.gz 0600L
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
? If you have following the suggestion of Michale Schwend you should remove
the reference to Petr Vanek from your Changelog entry

Bad:
- Rpmlint has the following cmplaints to the binary rpms:
rpmlint lastfmlib-0.4.0-1.fc14.x86_64.rpm 
lastfmlib.x86_64: W: spelling-error Summary(en_US) scrobbling -> scribbling, scrabbling
lastfmlib.x86_64: W: spelling-error %description -l en_US scrobble -> scribble, scrabble
lastfmlib.x86_64: E: non-standard-dir-perm /usr/share/doc/lastfmlib-0.4.0 0644L
lastfmlib.x86_64: E: zero-length /usr/share/doc/lastfmlib-0.4.0/TODO
1 packages and 0 specfiles checked; 2 errors, 2 warnings.
Please remove the empty TODO file and change the permissions of the documentation 
directory because traversal into this directory will be denied for ono-root users

I uploaded fixed SPEC and SRPM. I removed the reference to original spec from changelog, fixed source tarball permissions, removed the empty TODO and changed documentation directory permissions to 755. 
The spelling of "scrobbling/scrobble" is correct.

Spec URL: http://www.tepsonic.org/files/fedora/lastfmlib.spec
SRPM URL: http://www.tepsonic.org/files/fedora/lastfmlib-0.4.0-1.fc14.src.rpm

Good:
+ remove empty TODO file
* Local build works fine
* Package contains no patches
* Files permissions are ok on %files and %doc stanza

Bad:
- Please remove the BRs to automake and autoconf. Automake is only requires, if you make a patch to the Makefile.am file and want to recreate the makefile.in file. autoconf is only required, if you want to patch the configure.ac or configure.in file and recreating the configure script.

Please bump the release number of your package, if you are uploading a new release of your package for review.

Fixed and version bumped. 

Spec URL: http://www.tepsonic.org/files/fedora/lastfmlib.spec
SRPM URL: http://www.tepsonic.org/files/fedora/lastfmlib-0.4.0-2.fc14.src.rpm

Good:
- BR automake and autoconf was remove from SPEC file
+ Local build works fine
+ Koji build works fine.

*** APPROVED ***

Please let me know your FAS userid, so I can sponsor your.

If you have any issues don't hastle to contact me.

My FAS username is progdan.

Thank you!

You may create a SCM-Admin request for your package now.

New Package SCM Request
=======================
Package Name: lastfmlib
Short Description: a library providing implementation of LastFm Submission Protocol v1.2. It allows you to scrobble your tracks on Last.fm.
Owners: progdan
Branches: f14
InitialCC:

This ticket is not assigned to anyone.  Review tickets should be assigned to
the reviewer.  Please fix and re-set the fedora-cvs flag.

Sorry, I have forgotten it.

Git done (by process-git-requests).

lastfmlib-0.4.0-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/lastfmlib-0.4.0-2.fc14

lastfmlib-0.4.0-2.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update lastfmlib'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/lastfmlib-0.4.0-2.fc14

lastfmlib-0.4.0-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.