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=568833 Nils Philippsen <nphilipp@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ASSIGNED --- Comment #15 from Nils Philippsen <nphilipp@xxxxxxxxxx> 2010-04-01 13:14:08 EDT --- I'll simply work through the points at http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this package. Items marked "CHECK" aren't covered by the guidelines but you should check and fix them anyway in my opinion. Items marked "BAD" violate the guidelines in some point and need to be fixed. - GOOD: rpmlint on source package passes: nils@gibraltar:~/devel/fedora-review/OpenLP> rpmlint OpenLP-1.9.1-1.src.rpm OpenLP.src: W: invalid-url Source0: http://downloads.sourceforge.net/OpenLP/OpenLP-1.9.1.tar.gz HTTP Error 302: The HTTP server returned a redirect error that would lead to an infinite loop. The last 30x error message was: Found 1 packages and 0 specfiles checked; 0 errors, 1 warnings. -> I've downloaded the tarball earlier and manually verified that the source RPM is the same as upstream. - BAD: rpmlint on noarch package prints warnings/errors: nils@gibraltar:~/devel/fedora-review/OpenLP> rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/OpenLP-1.9.1-1.noarch.rpm OpenLP.noarch: W: file-not-utf8 /usr/share/doc/OpenLP-1.9.1/pyqt-sql-py2exe.txt OpenLP.noarch: E: non-executable-script /usr/lib/python2.6/site-packages/openlp/plugins/remotes/remoteclient.py 0644 /usr/bin/env OpenLP.noarch: W: hidden-file-or-dir /usr/lib/python2.6/site-packages/openlp/.version 1 packages and 0 specfiles checked; 1 errors, 2 warnings. -> convert the doc file to UTF-8 before installing -> remove the interpreter hashbang line from the non-executable python module -> The way openlp determines its version seems awfully complicated to me. If I were in your place, rather than putting it in a hidden file in the python module directory, I'd store it as a normal python variable in an openlp.version module which could be generated while installing (there you could check for bazaar versions and whatnot). - GOOD: package is named according to the Package Naming Guidelines - GOOD: spec file name matches package name - CHECK: The summary is really long, probably get rid of "Open Source" (kinda implied in Fedora). - BAD/CHECK: The description contains information not necessary for someone installing the Fedora package (we didn't have the old openlp.org software, neither is there Powerpoint on Fedora). You could clarify and condense this a good bit (and avoid any trademark issues) if it were phrased like this (wrapped at 80 chars/line of course): OpenLP is a church presentation and lyrics projection program, used to display song texts, Bible verses, videos and images using a computer and a beamer. I don't know if it can use openoffice.org for displaying presentations, if yes, just re-add that part (preferably without mentioning Powerpoint). - GOOD: package licensed properly - GOOD: license field matches actual license (GPLv2) - GOOD: license text included in package - GOOD: spec file written in American English - GOOD: spec file is legible - GOOD: source RPM tarball matches upstream - GOOD: package builds in mock - GOOD: all build requirements listed - PASS: No locales included (see below, though). - PASS: no libraries - GOOD: no bundled system libraries - GOOD: no relocation - BAD: Doesn't own all directories it creates: /usr/share/icons/hicolor/... are contained in the hicolor-icon-theme package -> please add hicolor-icon-theme as a dependency. - GOOD: doesn't list files more than once - GOOD: permissions set properly - GOOD: has %clean - GOOD: consistently uses macros - GOOD: package contains code - PASS/CHECK: no large documentation. You might want to include only files that have actual content, though -- PluginDevelopersGuide.txt is pretty empty. Feel free to leave this in if you're confident that this will get filled in time for 2.0 ;-). - GOOD: %doc files don't affect runtime - PASS: no header files - PASS: no static libraries - PASS: no devel libraries - PASS: no -devel subpackage - GOOD: no libtool archives - GOOD: contains desktop file, gets installed properly - GOOD: cleans buildroot at %install - GOOD: all filenames are valid UTF-8 Following some other things I noticed that you might want to check even though they're not covered by guidelines: - CHECK: It would be good if the package was localized, i.e. contained translations so people speaking other languages could use it. - CHECK: I think you should add a disttag to your release, e.g. "1%{?dist}", so that you can build the same version/release for different Fedora versions. Mind that you could have different python directories/major versions or need different other packages in different Fedora versions. - CHECK: The python executables use "#!/usr/bin/env python" as the interpreter. While there exists no guideline against using this rather than the regular "#!/usr/bin/python" it makes behavior of the program unpredictable if people e.g. install python 3 manually to /usr/local/bin. - CHECK: The executable is called "openlp.pyw". Why that extension? IMO, it would be totally sufficient if the file was called "openlp" without extension. That the package uses python is just an implementation detail. -- 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