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

fix: use BigInt when calculating nanos in Timestamp.fromMillis() #1468

Merged
merged 4 commits into from Apr 12, 2021

Conversation

thebrianchen
Copy link

After porting the original change to the web SDK, I noticed that the nanos calculation is subject to floating point precision loss. I used Math.floor() on the Web SDK, but using BigInt is more accurate, so I prefer to use it where we can.

@thebrianchen thebrianchen self-assigned this Apr 8, 2021
@thebrianchen thebrianchen requested review from a team as code owners April 8, 2021 15:50
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Apr 8, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 8, 2021
@thebrianchen
Copy link
Author

Build is broken b/c of sinon update, but will merge once it's fixed.

@schmidt-sebastian schmidt-sebastian removed their assignment Apr 8, 2021
@thebrianchen thebrianchen added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 12, 2021
@thebrianchen thebrianchen self-assigned this Apr 12, 2021
@thebrianchen thebrianchen added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 12, 2021
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Apr 12, 2021
@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #1468 (09f685a) into master (db989fc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1468      +/-   ##
==========================================
+ Coverage   98.20%   98.21%   +0.01%     
==========================================
  Files          32       32              
  Lines       19569    19567       -2     
  Branches     1285     1373      +88     
==========================================
  Hits        19217    19217              
+ Misses        348      346       -2     
  Partials        4        4              
Impacted Files Coverage Δ
dev/src/timestamp.ts 100.00% <100.00%> (ø)
dev/src/v1beta1/firestore_client.ts 97.74% <0.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db989fc...09f685a. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants