Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepend leading zeros #7663

Merged

Conversation

gleeper
Copy link
Contributor

@gleeper gleeper commented Apr 4, 2019

rfc3339 with nanoseconds wasn't prepending leading zeros so if an object had less than 100,000,000 nanoseconds, the nanoseconds string field would be incorrectly set.

@gleeper gleeper requested a review from crwilcox as a code owner April 4, 2019 17:54
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 4, 2019
@crwilcox
Copy link
Contributor

crwilcox commented Apr 5, 2019

@gleeper I cannot find specifics about this in the RFM specification. I took time to compare it to the implementation in Go, time.Parse(time.RFC3339Nano, "2006-01-02T15:04:05.7010Z") which will format as 2006-01-02T15:04:05.701Z

This seems to match our existing behavior.

@gleeper
Copy link
Contributor Author

gleeper commented Apr 5, 2019

I made a fix for prepending zeros, not appending zeros. DatetimeWithNanoseconds currently changes 2006-01-02T15:04:05.00123Z" to 2006-01-02T15:04:05.123Z". It's changing the nanoseconds field by removing leading zeros (and therefore multiplying the nanoseconds value by 10 or 100 or 1000 etc.), not trailing zeros

@crwilcox
Copy link
Contributor

crwilcox commented Apr 5, 2019

Sorry for the confusion @gleeper and maybe I could have been clearer. From what I could tell our behavior properly matched how things were happening in other languages.

I think I understand now the issue. We have have handling on the parsing side to shift incoming values. So when you provide .7 it becomes 700000000 nanoseconds. While this is done also with leading 0s so .007 becomes 7000000 we would return that as the string .7 and not .007 as expected.

One of your tests (the one removing trailing but adding leading) threw me a bit and sort of made me wonder what you were trying to do :)

@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 5, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 5, 2019
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 8, 2019
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 8, 2019
@crwilcox crwilcox merged commit a776beb into googleapis:master Apr 8, 2019
@crwilcox
Copy link
Contributor

crwilcox commented Apr 8, 2019

@gleeper thanks for the contribution to the project. Your change has been merged and will ship in the next release.

@gleeper
Copy link
Contributor Author

gleeper commented Apr 8, 2019

@crwilcox Happy to help! Do you have an estimate as to when the next release will be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants