Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1463)

Issue 1698050: issue5288: tzinfo objects with sub-minute offsets are not supported (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by sasha
Modified:
9 years, 1 month ago
Reviewers:
Benjamin
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Patch Set 1 #

Total comments: 17
Unified diffs Side-by-side diffs Delta from patch set Stats (+387 lines, -466 lines) Patch
M Modules/datetimemodule.c View 25 chunks +387 lines, -466 lines 17 comments Download

Messages

Total messages: 6
Benjamin
It looks good except for a few nits and a lack of tests. http://codereview.appspot.com/1698050/diff/1/2 File ...
9 years, 1 month ago (2010-07-07 22:04:33 UTC) #1
sasha
On Wed, Jul 7, 2010 at 6:04 PM, <musiccomposition@gmail.com> wrote: > It looks good except ...
9 years, 1 month ago (2010-07-07 22:12:21 UTC) #2
benjamin_python.org
2010/7/7 Alexander Belopolsky <alexander.belopolsky@gmail.com>: > On Wed, Jul 7, 2010 at 6:04 PM, <musiccomposition@gmail.com> wrote: ...
9 years, 1 month ago (2010-07-07 22:13:22 UTC) #3
sasha
Thanks for the review. I have a couple of questions about your comments. Please see ...
9 years, 1 month ago (2010-07-07 22:29:43 UTC) #4
benjamin_python.org
2010/7/7 <alexander.belopolsky@gmail.com>: > Thanks for the review. I have a couple of questions about your ...
9 years, 1 month ago (2010-07-07 22:34:05 UTC) #5
sasha
9 years, 1 month ago (2010-07-07 23:16:45 UTC) #6
http://codereview.appspot.com/1698050/diff/1/2
File Modules/datetimemodule.c (right):

http://codereview.appspot.com/1698050/diff/1/2#newcode880
Modules/datetimemodule.c:880: offset = PyObject_CallMethod(tzinfo, name, "O",
tzinfoarg);
On 2010/07/07 22:04:34, Benjamin wrote:
> You can just use PyObject_CallMethodObjArgs.

No, I cannot.   PyObject_CallMethodObjArgs needs a PyObject * as a method name
and I have a char *.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b