Code review comment for lp:~leonardr/wadllib/convert-datetime-309950

Revision history for this message
Gary Poster (gary) wrote :

*merge-conditional (Edwin)

Looks good to me. Thank you.

I saw two spelling errors and I suggest we rearrange the regex comments.

In application.py:

> + # Raise an unadored ValueError so the client
> + # can treat the value as a string if they
> + # want.

s/unadored/unadorned/

and in iso_strptime.py:

> + (?P<year>\d{4})\-(?P<month>\d{2})\-(?P<day>\d{2})
> + # pattern matching date
> + T
> + # seperator
> + (?P<hour>\d{2})\:(?P<minutes>\d{2})\:(?P<seconds>\d{2})
> + # pattern matching time
> + (\.(?P<microseconds>\d{6}))?
> + # pattern matching optional microseconds
> + (?P<tz_offset>[\-\+]\d{2}\:\d{2})?
> + # pattern matching optional timezone offset

s/seperator/separator/

Also I think the comments should now go before the pertinent part of the pattern:

+ # pattern matching date
+ (?P<year>\d{4})\-(?P<month>\d{2})\-(?P<day>\d{2})
+ # separator
+ T
+ # pattern matching time
+ (?P<hour>\d{2})\:(?P<minutes>\d{2})\:(?P<seconds>\d{2})
+ # pattern matching optional microseconds
+ (\.(?P<microseconds>\d{6}))?
+ # pattern matching optional timezone offset
+ (?P<tz_offset>[\-\+]\d{2}\:\d{2})?

Gary

review: Approve (code)

« Back to merge proposal