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

Issue 7153043: mtime_override for 1.9 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by ReimarBauer
Modified:
2 years, 1 month ago
Reviewers:
thomas.j.waldmann
Visibility:
Public.

Description

mtime_override for 1.9

Patch Set 1 #

Total comments: 8

Patch Set 2 : mtime in UTC for 1.9 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -7 lines) Patch
M MoinMoin/PageEditor.py View 1 5 chunks +11 lines, -4 lines 0 comments Download
M MoinMoin/script/import/wikipage.py View 1 3 chunks +16 lines, -3 lines 0 comments Download

Messages

Total messages: 1
Thomas.J.Waldmann
2 years, 1 month ago (2013-01-17 16:08:56 UTC) #1
https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py
File MoinMoin/PageEditor.py (right):

https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py#newcode91
MoinMoin/PageEditor.py:91: @keyword mtime_override: override mtime (default
None)
please also document what None means.

how about just naming it mtime? using current time, if not given would be quite
natural.

https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py#newcode99
MoinMoin/PageEditor.py:99: self.mtime_override = keywords.get('mtime_override',
None)
None is the default for .get(), you don't need to give it

https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py#newcode1008
MoinMoin/PageEditor.py:1008: if self.mtime_override:
... is not None:

https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py#newcode1009
MoinMoin/PageEditor.py:1009: mtime_usecs = self.mtime_override
i would not have mtime(_override) as usecs, but rather same unit of measure and
type as os.path.getmtime() returns.

and then do the wikiutil.timestamp2version() call after the if-block.

https://codereview.appspot.com/7153043/diff/1/MoinMoin/PageEditor.py#newcode1018
MoinMoin/PageEditor.py:1018: mtime_usecs =
wikiutil.timestamp2version(time.time())
see above. somehow looks duplicated though.

https://codereview.appspot.com/7153043/diff/1/MoinMoin/script/import/wikipage.py
File MoinMoin/script/import/wikipage.py (right):

https://codereview.appspot.com/7153043/diff/1/MoinMoin/script/import/wikipage...
MoinMoin/script/import/wikipage.py:44: help='Use TIME (YYYY-MM-DD HH:MM:SS) for
edit history / RecentChanges')
you should also say what it does if this is not used, then it might get clearer.
also, it should be UTC if there is no timezone specified.

https://codereview.appspot.com/7153043/diff/1/MoinMoin/script/import/wikipage...
MoinMoin/script/import/wikipage.py:79: mtime =
timestamp2version(time.mktime(time.strptime(self.options.mtime, "%Y-%m-%d
%H:%M:%S")))
be careful, iirc mktime is the inverse operation of localtime, NOT of gmtime.

https://codereview.appspot.com/7153043/diff/1/MoinMoin/script/import/wikipage...
MoinMoin/script/import/wikipage.py:81: mtime = timestamp2version(time.time())
but this IS utc
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1437:571d95b2634e