Skip to content

Add functions to get milli/micro/nano-seconds from a DateTime#81

Merged
lifthrasiir merged 2 commits intochronotope:masterfrom
beneills:master
Jul 16, 2016
Merged

Add functions to get milli/micro/nano-seconds from a DateTime#81
lifthrasiir merged 2 commits intochronotope:masterfrom
beneills:master

Conversation

@beneills
Copy link
Copy Markdown
Contributor

Using the underlying naive::NaiveTime fractional part, we compute
the number of milli/micro/nano-seconds since the last second boundary.

The reason for not computing elapsed time since 1970 is because we
would hit potential issues of i64s not being large enough (the range
would be strictly smaller than the 64bit-timestamp range, causing
compatibility issues).

This was written in response to issue #74.

Using the underlying naive::NaiveTime fractional part, we compute
the number of milli/micro/nano-seconds since the last second boundary.

The reason for not computing elapsed time since 1970 is because we
would hit potential issues of i64s not being large enough (the range
would be strictly smaller than the 64bit-timestamp range, causing
compatibility issues).
@lifthrasiir
Copy link
Copy Markdown
Contributor

lifthrasiir commented Jul 15, 2016

I think it is a fine addition, but I have some questions:

  • Shouldn't the names of those methods follow Duration::subsec_nanos? (My opinion: Yes.)
  • How should they behave on the leap seconds? The nanosecond part may exceed 999,999,999 in that case, and you need a clear policy for that. (My opinion: Let they return the value exceeding 1s, and signal that caveat loudly. The proper formatting should go through format which already handles leap seconds correctly, and the accessors should be considered as a "raw" format.)
  • Probably nitpicking, but please avoid adding spaces in irrelevant lines ;) You can surgery them with git add -p or similar tools.

Renamed accessors to subsec_{nano,micro,milli}, as suggested
in pull request comment.  Also added warnings for leap second
consitions causing these values to exceed the normal range
of 0..10^n.

Fixed editor's previous obnoxious whitespace changes.
@beneills
Copy link
Copy Markdown
Contributor Author

beneills commented Jul 15, 2016

Shouldn't the names of those methods follow Duration::subsec_nanos? (My opinion: Yes.)

Renamed according to this schema.

How should they behave on the leap seconds? The nanosecond part may exceed 999,999,999 in that case, and you need a clear policy for that. (My opinion: Let they return the value exceeding 1s, and signal that caveat loudly. The proper formatting should go through format which already handles leap seconds correctly, and the accessors should be considered as a "raw" format.)

Added prominent Rustdoc warnings to all accessors.

Probably nitpicking, but please avoid adding spaces in irrelevant lines ;) You can surgery them with git add -p or similar tools.

Fixed. Apologies for this: was trying out a new editor.

@lifthrasiir lifthrasiir merged commit bb50154 into chronotope:master Jul 16, 2016
@lifthrasiir
Copy link
Copy Markdown
Contributor

@beneills I've squashed and merged the PR. Thank you!

lifthrasiir added a commit that referenced this pull request Aug 2, 2016
- Tons of documentation updates! (#77, #78, #80, #82 and my own
  changes as well)

- `DateTime::timestamp_subsec_{millis,micros,nanos}` methods have
  been added. (#81)

- When the system time records a leap second,
  the nanosecond component was mistakenly reset to zero. (#84)

- `Local` offset misbehaves in Windows for August and later,
  due to the long-standing libtime bug (dates back to mid-2015).
  Workaround has been implemented. (#85)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants