Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=585607 Jason Tibbitts <tibbs@xxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |tibbs@xxxxxxxxxxx Flag| |fedora-review? --- Comment #3 from Jason Tibbitts <tibbs@xxxxxxxxxxx> 2010-11-22 22:33:39 EST --- OK, here's a review. Unfortunately there is significant work to be done before this package can be acceptable. Note that if you're not targeting RHEL4 or 5, you can remove BuildRoot:, %clean and the first line of %install. Note that your manual "Requires: perl" is unnecessary; rpmlint already finds the perl dependency. (Actually it finds four of them, but that's not really your problem.) I was going to comment on the lack of indentation in your %pre script making it tough to read, but it looks like the example from the UsersAndGroups page somehow lost its indentation; I fixed it. Your license tag seems to be incorrect. All of the source files which have the proper GPL license header indicate GPLv2+. Unpack the tarball and see: grep -rC1 'of the License' . (This doesn't count the improperly bundled libraries, which are a separate issue.) /etc/logrotate.d/gnump3d needs to be marked %config(noreplace) as admins do edit these files and will expect them to be preserved. rpmlint complains about this: gnump3d.noarch: W: non-conffile-in-etc /etc/logrotate.d/gnump3d It is indeed in the right place; rpmlint is properly warning that it's not marked properly as a config file. I've no way to properly test this as all my music is FLAC. I guess I could convert some of it, but it's taken long enough to review this as it is. The file lib/gnump3d/WMA.pm seems to be a bundled copy of http://search.cpan.org/~daniel/Audio-WMA-1.3/. This is a blocker. It also makes me wonder what else they're bundling; a quick check shows IP.pm, MD5.pm and mp3info.pm are ripped off from some other packages as well. It's nice that they say the software is self-contained, but achieving that through bundling is not proper and must be fixed. * source files match upstream. sha256sum: 1ac5bd0e850b0e18ccd9d19219f5108fa84b50d8ae3825a361e8b907eab7f19c gnump3d-3.0.tar.bz2 * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. X license field should be GPLv2+ * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper (none). * package builds in mock (rawhide, x86_64). * package installs properly. X rpmlint has a valid complaint. * final provides and requires are sane: config(gnump3d) = 3.0-4.fc15 perl(gnump3d::IP) = 3.14 perl(gnump3d::MD5) = 1.6 perl(gnump3d::Request) perl(gnump3d::WMA) = 0.7 perl(gnump3d::base64) perl(gnump3d::config) = 0.01 perl(gnump3d::files) perl(gnump3d::filetypes) perl(gnump3d::lang::lookup) perl(gnump3d::mp3info) = 1.13 perl(gnump3d::mp4info) = 1.03 perl(gnump3d::ogginfo) = 0.01 perl(gnump3d::readtags) perl(gnump3d::sorting) = 0.01 perl(gnump3d::tagcache) perl(gnump3d::url) = 0.01 perl(plugins::copying) = 1..2 perl(plugins::info) = 1..2 perl(plugins::now) = 1..2 perl(plugins::playlist) = 1..2 perl(plugins::prefs) = 1..2 perl(plugins::random) = 1..2 perl(plugins::recent) = 1..2 perl(plugins::search) = 1..2 perl(plugins::size) = 1..2 perl(plugins::stats) = 1..2 perl(plugins::tagbrowse) = 1..2 gnump3d = 3.0-4.fc15 = /bin/bash /bin/sh /sbin/chkconfig /sbin/service /usr/bin/perl config(gnump3d) = 3.0-4.fc15 ? perl <------- unneeded manual dependency perl >= 0:5.004 perl >= 0:5.005 perl >= 0:5.005_62 perl >= 0:5.006 perl(AutoLoader) perl(Carp) perl(Encode) perl(Encode::Guess) perl(English) perl(Env) perl(Exporter) perl(File::Find) perl(Getopt::Long) perl(IO::File) perl(IO::Socket) perl(POSIX) perl(Pod::Usage) perl(Socket) perl(Symbol) perl(gnump3d::IP) perl(gnump3d::MD5) perl(gnump3d::Request) perl(gnump3d::WMA) perl(gnump3d::config) perl(gnump3d::files) perl(gnump3d::filetypes) perl(gnump3d::lang::lookup) perl(gnump3d::mp3info) perl(gnump3d::mp4info) perl(gnump3d::ogginfo) perl(gnump3d::readtags) perl(gnump3d::sorting) perl(gnump3d::tagcache) perl(gnump3d::url) perl(integer) perl(overload) perl(sigtrap) perl(strict) perl(vars) perl(warnings) shadow-utils X contains many bundled libraries. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files. * scriptlets are OK (user/group creation). * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review