Bug 741945 - Review Request: python-isodate - An ISO 8601 date/time/duration parser and formater
Summary: Review Request: python-isodate - An ISO 8601 date/time/duration parser and fo...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mo Morsi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 743402
TreeView+ depends on / blocked
 
Reported: 2011-09-28 14:56 UTC by James Laska
Modified: 2013-09-02 06:57 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-12-08 12:44:59 UTC
Type: ---
mmorsi: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

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).


Note You need to log in before you can comment on or make changes to this bug.