This is an archive of the discontinued Mercurial Phabricator instance.

util: add `hgdatetopython` to convert hg-style dates to datetimes
AbandonedPublic

Authored by phillco on Aug 13 2017, 11:06 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Summary

Extract the existing code from `datestr`.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

phillco created this revision.Aug 13 2017, 11:06 PM
lothiraldan added a subscriber: lothiraldan.EditedAug 14 2017, 5:40 AM

Small nit-picking style comments on the doc-string.

I like this series, nice addition!

mercurial/util.py
1862

Usually doc-string start with a lowercase letter in Mercurial code

1898

Why not passing the date and tz as separate argument instead of a tuple?

Forget about my comment on the commit message, I was fooled by phabricator

Forget about my comment on the commit message, I was fooled by phabricator

Did you make one? I only see the two inline ones.

mercurial/util.py
1898

I think usually Mercurial dates as passed around as tuples everywhere, so I decided to maintain that convention.

durin42 added inline comments.
mercurial/util.py
1862

This function makes me really nervous, because it's a naive datetime rather than a tz-aware datetime. Could we do better about preserving the exact point-in-time value of the tz-aware time information?

("no" is a possible answer here, but I'd really like to know why we're not storing the zone offset in the datetime object.)

phillco added inline comments.Aug 15 2017, 2:03 PM
mercurial/util.py
1862

Hm, good question. I only do that because that's what the earlier code did -- that change dates from 87c6ad2251d8703d7f16d51e99e3d1c040f0be49, I think.

phillco added a comment.EditedAug 15 2017, 2:03 PM
This comment has been deleted.
mercurial/util.py
1862

87c6ad2251d8703d7f16d51e99e3d1c040f0be49 -- it linkifies if you don't use backtics, apparently

durin42 requested changes to this revision.Aug 28 2017, 2:52 PM
durin42 added inline comments.
mercurial/util.py
1862

So, having reflected on this a bit: I'm too wary of the naive datetime. I think the easiest fix for this would be for us to have our own datetime.tzinfo implementation and then include that when we construct the datetime so we don't lose the zone offset information.

This revision now requires changes to proceed.Aug 28 2017, 2:52 PM
phillco abandoned this revision.Sep 1 2017, 1:35 PM

I'll revisit later.