Bug 741945

Summary:	Review Request: python-isodate - An ISO 8601 date/time/duration parser and formater
Product:	[Fedora] Fedora	Reporter:	James Laska <jlaska>
Component:	Package Review	Assignee:	Mo Morsi <mmorsi>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	jturner, mmorsi, notting, package-review
Target Milestone:	---	Flags:	mmorsi: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2011-12-08 12:44:59 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Bug Depends On:
Bug Blocks:	743402

Description James Laska 2011-09-28 14:56:20 UTC

Spec URL: http://jlaska.fedorapeople.org/repos/review/python-isodate.spec
SRPM URL: http://jlaska.fedorapeople.org/repos/review/python-isodate-0.4.4-1.fc15.src.rpm
Description: 

This module implements ISO 8601 date, time and duration parsing. The implementation follows ISO8601:2004 standard, and implements only date/time representations mentioned in the standard. If something is not mentioned there, then it is treated as non existent, and not as an allowed option.

For instance, ISO8601:2004 never mentions 2 digit years. So, it is not intended by this module to support 2 digit years. (while it may still be valid as ISO date, because it is not explicitly forbidden.) Another example is, when no time zone information is given for a time, then it should be interpreted as local time, and not UTC.

As this module maps ISO 8601 dates/times to standard Python data types, like date, time, datetime and timedelta, it is not possible to convert all possible ISO 8601 dates/times. For instance, dates before 0001-01-01 are not allowed by the Python date and datetime classes. Additionally fractional seconds are limited to microseconds. That means if the parser finds for instance nanoseconds it will round it to microseconds.


== rpmlint output ==

There are no errors, only spelling warnings which I believe can be ignored.

python-isodate.noarch: W: spelling-error Summary(en_US) formater -> for mater, for-mater, format er
python-isodate.noarch: W: spelling-error %description -l en_US datetime -> date time, date-time, daytime
python-isodate.noarch: W: spelling-error %description -l en_US timedelta -> time delta, time-delta, mealtime
python-isodate.src: W: spelling-error Summary(en_US) formater -> for mater, for-mater, format er
python-isodate.src: W: spelling-error %description -l en_US datetime -> date time, date-time, daytime
python-isodate.src: W: spelling-error %description -l en_US timedelta -> time delta, time-delta, mealtime
2 packages and 1 specfiles checked; 0 errors, 6 warnings.

Comment 1 Mo Morsi 2011-09-29 14:26:26 UTC

Taking this one

Comment 2 Mo Morsi 2011-09-29 14:31:02 UTC

Overall looks good, koji build is green

http://koji.fedoraproject.org/koji/taskinfo?taskID=3388155

A few nits

* BuildRoot is no longer needed and should be removed

* same w/ the rm buildroot in the install and clean sections

* Do we still need the exceptions for the older non-supported Fedora versions? I see these in the python guidelienss though so if they are required you can just ignore this note

* is python-devel required as a build time dependency (again see this in the python guideliness, but this is my first python package review so I could be mistaken)

From what I can tell, everything else looks good, so APPROVED

Comment 3 James Laska 2011-09-30 12:19:54 UTC

(In reply to comment #2)
> Overall looks good, koji build is green

Thanks for the review!
 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=3388155
> 
> A few nits
> 
> * BuildRoot is no longer needed and should be removed

Thanks, removed.  I also replaced the use of $RPM_OPT_FLAGS with %{optflags} for consistent macro use.

> * same w/ the rm buildroot in the install and clean sections

Looks like that isn't required for F13 and newer.  I'll probably leave this for EL6 packages, unless I can confirm that it's no longer needed there either.

> * Do we still need the exceptions for the older non-supported Fedora versions?
> I see these in the python guidelines though so if they are required you can
> just ignore this note

I left them mainly for EL5 or EL6 compat.  I could be pedantic, and they're just not needed anymore.  I'll try to confirm.

> * is python-devel required as a build time dependency (again see this in the
> python guideliness, but this is my first python package review so I could be
> mistaken)

Excellent catch.  Normally, yes.  The package python-devel provides the distutils library which is needed by many python packages that 'import distutils' from inside their setup.py.  In the case of isodate, it's doing 'from setuptools import setup'.  For that, I've added a BuildRequires: python-setuptools.

*BUT*, I missed that I'm relying on distutils for the %{python_sitelib} macro.  So it is needed.  Thanks!

> From what I can tell, everything else looks good, so APPROVED

Thanks again.  I updated the .spec and src.rpm posted from comment#0, and will request SCM shortly.

Comment 4 James Laska 2011-09-30 12:21:17 UTC

Mo: anyone else you can think of who would like to be an owner/cc of this package?

Comment 5 Mo Morsi 2011-09-30 12:27:34 UTC

Hrm not sure, since its a pulp dependency and not an aeolus one,probably don't have the cycles to take it on my plate. Perhaps ping'ing one of the pulp guys/gals?

Comment 6 James Laska 2011-09-30 12:56:38 UTC

New Package SCM Request
=======================
Package Name: python-isodate
Short Description: An ISO 8601 date/time/duration parser and formater
Owners: jlaska jmatthews
Branches: f16
InitialCC:

Comment 7 Gwyn Ciesla 2011-09-30 13:27:38 UTC

Git done (by process-git-requests).

Comment 8 Fedora Update System 2011-10-04 19:46:31 UTC

python-isodate-0.4.4-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/FEDORA-2011-13548

Comment 9 Fedora Update System 2011-10-09 19:39:37 UTC

python-isodate-0.4.4-1.fc16 has been pushed to the Fedora 16 stable repository.

Comment 10 Mo Morsi 2011-12-08 12:44:59 UTC

Closing this out as this is now in Fedora

Comment 11 James Laska 2012-01-24 14:12:38 UTC

Package Change Request
======================
Package Name: python-isodate
New Branches: el6 f15
Owners: jlaska

Comment 12 Gwyn Ciesla 2012-01-24 14:16:08 UTC

Git done (by process-git-requests).