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=566411 Kalev Lember <kalev@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |kalev@xxxxxxxxxxxx AssignedTo|nobody@xxxxxxxxxxxxxxxxx |kalev@xxxxxxxxxxxx Flag| |fedora-review? --- Comment #3 from Kalev Lember <kalev@xxxxxxxxxxxx> 2010-03-12 17:40:20 EST --- Taking for review. The stated "License: GPLv2" is wrong. Some of the files appear to be licensed under GPLv2+, and others are LGPLv2+. The license tag should thus read: License: GPLv2+ and LGPLv2+ As per https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net , you should use the following for source URL: http://downloads.sourceforge.net/umit/umit-%{version}%{prerelease}.tar.gz The package uses python gtk2 bindings and needs to require pygtk2 to make sure it is installed at runtime. You've split html documentation into -doc subpackage. I think it would be better to have this in the main package. Help->Help is supposed to open the html pages, but if -doc subpackage is not installed, the user gets an error dialog instead. It'd be better user experience if everything worked out of the box. Also, anything marked with %doc must not affect the runtime of the application (http://fedoraproject.org/wiki/Packaging/Guidelines#Documentation), so please don't mark the html documentation with %doc. Various minor issues -------------------- %python_sitearch macro defined at the top of the file isn't used anywhere. > %patch0 -p1 -b .setup.py The patch -b option is for making backups of the original files. Patch0 is a patch to remove two lines from setup.py file. When the patch command runs, it thus first creates a backup file called setup.py.setup.py which doesn't make much sense. I'd suggest to use something like "%patch0 -p1 -b .check-buildroot", so that the backup file would get named as "setup.py.check-buildroot". I would suggest to not use %{__command} macros in new spec files, as they are really not needed and only make the spec file harder to read. Instead of %{__chmod}, %{__rm}, %{__sed}, %{__ln_s}, and %{__install} I'd use just plain chmod, rm, sed, ln -s, and install. However, %{__python} might be useful and should remain as macro. Also see how %__ macros are used in the official Fedora python packaging guidelines: http://fedoraproject.org/wiki/Packaging:Python But this is mostly just personal preference, and up to you if you want to change that. -- 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