[Bug 531544] Review Request: python-trml2pdf - Tiny RML2PDF is a tool to easily create PDF documents without programming

[bugzilla at redhat.com]

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 at yahoo.com> 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.