[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