Skip to content

Add support of timestamps in nanoseconds#365

Merged
adonovan merged 4 commits intogoogle:masterfrom
essobedo:364/support-nano-in-timestamp
Apr 6, 2021
Merged

Add support of timestamps in nanoseconds#365
adonovan merged 4 commits intogoogle:masterfrom
essobedo:364/support-nano-in-timestamp

Conversation

@essobedo
Copy link
Copy Markdown
Contributor

@essobedo essobedo commented Mar 20, 2021 •
edited
Loading

fixes #364

Motivation

For many real life use cases, it is important to be a able to support timestamps expressed in nanoseconds.

Modifications:

  • Overloads the function from_timestamp to be able to set nanoseconds.
  • Fixes the documentation of the built in function from_timestamp
  • Improves the documentation (not related to this issue)
  • Adds a test for the function now since it was not covered by any tests (not related to this issue)

@essobedo essobedo mentioned this pull request Mar 20, 2021
Closed
@essobedo
Copy link
Copy Markdown
Contributor Author

essobedo commented Mar 20, 2021

Hi @adonovan and @brandjon WDYT of this one?

@essobedo
Copy link
Copy Markdown
Contributor Author

essobedo commented Mar 20, 2021

Please note that I intentionally avoided to overload the function from_timestamp for clarity and simplicity

adonovan
adonovan requested changes Mar 20, 2021
View reviewed changes
@essobedo essobedo requested a review from adonovan March 20, 2021 16:52
adonovan
adonovan reviewed Mar 22, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adonovan adonovan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@essobedo essobedo requested a review from adonovan March 22, 2021 18:17
adonovan
adonovan approved these changes Mar 22, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adonovan adonovan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandjon LGTM, ready for you to merge.

@essobedo
Copy link
Copy Markdown
Contributor Author

essobedo commented Mar 25, 2021

@brandjon Is-this PR good enough to be merged?

@adonovan adonovan requested a review from brandjon March 25, 2021 21:28
@essobedo
Copy link
Copy Markdown
Contributor Author

essobedo commented Apr 2, 2021

@brandjon Could you please at least tell me if this project won't be maintained anymore? Because I wanted to work on #241 during my spare time, once this PR will be merged, if this project is no more supported/maintained I won't do it. Thank you in advance for your answer 🙏

@adonovan adonovan merged commit f54874d into google:master Apr 6, 2021
@brandjon
Copy link
Copy Markdown
Collaborator

brandjon commented Apr 6, 2021

Apologies for letting this slip. Alan has write access to this repo and I've confirmed with him that he can merge.

@essobedo essobedo deleted the 364/support-nano-in-timestamp branch April 6, 2021 18:01
This was referenced Apr 8, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adonovan adonovan adonovan approved these changes

@brandjon brandjon Awaiting requested review from brandjon

+1 more reviewer

@ssoroka ssoroka ssoroka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@brandjon brandjon

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support of timestamps in nanoseconds

4 participants

@essobedo @brandjon @ssoroka @adonovan