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=508750 David A. Wheeler <dwheeler@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dwheeler@xxxxxxxxxxxx --- Comment #7 from David A. Wheeler <dwheeler@xxxxxxxxxxxx> 2010-01-25 10:15:08 EST --- Hi, I've started an initial review. First, overall comments: rpmlint reports this on the spec file: trash-cli.spec: W: no-buildroot-tag It's true that buildroot isn't used in *this* version of Fedora, but if this package is copied elsewhere it could lead to disaster. Could you please add a buildroot tag? It's not a requirement, but I suggest appending "/" for all items in the "%files" section that are directories. That makes it really obvious which entries are directories. According to: http://fedoraproject.org/wiki/Packaging/Python/Eggs The invocation of "... setup.py install" should use "%{__python}" instead of "python". I did an "rpmbuild -bp" and didn't find any .pyo or .pyc files (good). After building, rpmls looks okay. You could simplify the spec file a little by changing %files to just include: %{python_sitelib}/* That would collapse 2 %files entries to one, and then you wouldn't need the "pyver" macro at all. The license looks correct. The included PKG-INFO and setup.py files say that the liecnse is "GPL v2", and the included file COPYING is the text of GPL v2. I ran mock (mock --rebuild ~/rpmbuild/SRPMS/trash-cli-0.11.2-1.fc12.src.rpm), and it built without trouble. Good! The description text isn't quite right. You can't say "trash" to replace "rm", because there is no "/usr/bin/trash". Simiarly, the "trash-put" man page is just wrong; the man page says you're supposed to say "trash FILE", but this package requires that you say "trash-put FILE". I recommend that this package install a /usr/bin/trash, and that "man trash" also works. You can make /usr/bin/trash a symlink to trash-put, if you'd like. That way, the docs are right, *AND* it makes it easy for people to invoke (just replace "rm" with "trash"). -- 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