[Bug 722249] Review Request: python-hl7 - Python library parsing HL7 v2.x and v3.x messages
bugzilla at redhat.com
bugzilla at redhat.com
Thu Jul 21 18:07:14 UTC 2011
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=722249
--- Comment #8 from Ankur Sinha <sanjay.ankur at gmail.com> 2011-07-21 14:07:13 EDT ---
(In reply to comment #6)
> Hi Ankur,
>
> I've a few things that need addressing:
>
> * Missing %if ! (0%{?fedora} > 12 || 0%{?rhel} > 5)
> http://fedoraproject.org/wiki/Packaging:Python#Macros
The if condition isn't required any more, since we are always going to be
building for f>12 ;)
>
> * Description is longer than 80 characters, and you can probably remove the
> link as it is not relevant in this section.
Corrected.
>
> * Your BuildRequires section should be:
> BuildRequires: python2-devel
> BuildRequires: python-setuptools-devel
I created this spec using rpmdev-newspec -t python, which provides a skeleton
spec file for python packages. Not sure why the difference.
repoquery -i python2-devel doesn't return me anything. I'm letting this be for
the time being.
>
> * %defattr required in %files section.
It isn't after rpm 4.4
https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions
>
> * Missing %doc where you need to include LICENSE, README.rst etc.
>
> I noticed that SOURCES.txt and the MANIFEST list docs/* and tests/* which are
> not included in your package - I'm not sure if they were meant to be.
>
Corrected. I haven't included tests, they're not for users, more for upstream
IMO.
>
> cheers,
>
> Brendan
Fresh spec/srpm:
http://ankursinha.fedorapeople.org/hl7/python-hl7.spec
http://ankursinha.fedorapeople.org/hl7/python-hl7-0.2.0-2.fc15.src.rpm
* Thu Jul 21 2011 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.2.0-2
- Correct description
- Make additional docs
Thanks,
Ankur
--
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.
More information about the package-review
mailing list