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=531544 --- Comment #6 from Cristian Ciupitu <cristian.ciupitu@xxxxxxxxx> 2010-07-08 19:41:21 EDT --- First of all, I want to mention that I've updated the RPM in the meantime: Spec URL: http://github.com/ciupicri/rpmbuild/raw/master/SPECS/python-trml2pdf.spec SRPM URL: http://sites.google.com/site/cristianciupitu/python-trml2pdf-1.2-1.fc13.src.rpm (In reply to comment #5) > First of all, as a sponsor I think you need to do more informal reviews. I see > you have been already active otherwise, but that does not make up for review > experience. Ok, I'll try to help more with other reviews. > Reviewing is a very important part of being involved in Fedora packaging, and > there is no better way to show that you understand the guidelines. As a > reminder the most important ones are > http://fedoraproject.org/wiki/Packaging/Guidelines > http://fedoraproject.org/wiki/Packaging/ReviewGuidelines > Additionally to the Packaging Guidelines, there are a bunch of language / > application specific guidelines that are linked to in the Packaging Guidelines. > > Here are some tricks of the trade: > http://fedoraproject.org/wiki/Packaging_tricks > http://fedoraproject.org/wiki/Packaging/ScriptletSnippets > http://fedoraproject.org/wiki/Common_Rpmlint_issues Thanks. > Some comments about the package itself: > > - The name is incorrect. This is a tool, not a pure python library, so the name > should be just trml2pdf. My initial name was trml2pdf, but another Fedora packager suggested me something else: <abadger1999> ciupicri: Since it's a dependency, is it the python module or the program that's being used? <ciupicri> abadger1999, the python module <ciupicri> abadger1999, satchmo (django app) uses its library not its program <abadger1999> ciupicri: Ideally, upstream would split the script out into its own file and import the module to make the script run but it sounds like upstream is pretty dead. <abadger1999> So I'd do this: <abadger1999> link the trml2pdf.py file to %{_bindir}/trml2pdf (leave off the .py extension in bindir) <abadger1999> and name the package python-trml2pdf > - The summary and description conflict. Besides, the description being shorter > than the summary is quite nonorthodox. I'd change them both to something like > "A tool to convert Report Markup Language (RML) files to PDF" The summary was taken from the description field of "setup.py", but I do agree that the current situation is a bit odd and I'll try to use something better. > - URL is incorrect, it should be something of the sort > http://packages.pardus.org.tr/contrib/source/trml2pdf.html > (it should point to the package homepage, not the directory where the tarball > has been taken from) As far as I know the package is unmaintained, so that why I've used that URL. > - You don't need to use "%{__python}", plain "python" will do just fine. I know, but that's what rpmdev-newspec suggests. > - Don't use wildcards where they are not necessary: from the statement > %{python_sitelib}/* > it is not clear what actually gets included. In the case of Python this has > also practical implications - normally, in addition to the library also an egg > is produced. Using the wildcard prevents you from being notified that the egg > is missing. Please use more explicit statements such as > %{python_sitelib}/trml2pdf/ > %{python_sitelib}/trml2pdf-*.egg-info It's nice to have a more explicit list of the included files, but on the other hand I think that it makes things a bit harder to maintain. I've also seen a counter example: http://cvs.fedoraproject.org/viewvc/rpms/python-fedora/F-13/python-fedora.spec?view=markup . -- 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