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

[LANG-1444] NumberUtils.createNumber(), BigDecimal for decimal fractions tending to zero #675

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bndhanush
Copy link

NumberUtils.createNumber(), BigDecimal for decimal fractions tending to zero

NumberUtils.createNumber(), BigDecimal for decimal fractions tending to zero
@garydgregory
Copy link
Member

d0b02bt added 2 commits December 16, 2020 12:36
adding white spaces for ,
adding white spaces for ,
@bndhanush
Copy link
Author

bndhanush commented Dec 16, 2020

Hi @garydgregory,

Thank you for the notice. I have fixed the formatting issue. I see it is ok for the 16-ea build to fail, is that correct understanding?
I am pretty new to OS, can you please help me in this?

@coveralls
Copy link

coveralls commented Dec 16, 2020

Coverage Status

Coverage decreased (-0.1%) to 94.915% when pulling 548c412 on bndhanush:master into 615eee5 on apache:master.

@garydgregory garydgregory changed the title LANG-1444 [LANG-1444] NumberUtils.createNumber(), BigDecimal for decimal fractions tending to zero Dec 22, 2020
@garydgregory
Copy link
Member

@chtompki
May you please validate this one?

@bndhanush
Copy link
Author

@chtompki and @garydgregory.
Can you please help in checking if the PR is valid. Also suggest if any changes are required.

@garydgregory
Copy link
Member

Hi @bndhanush
Thank you for your PR.
In the new test, I do not think it is enough to check that an instance of right class is created, you want to check that the right value is created to make sure we do not have the opposite issue: loosing precision.

added test cases to ensure precision is not lost
@bndhanush
Copy link
Author

hi @garydgregory

Thanks for the input.
I added test case to ensure that precision is not lost and we maintain the value.

Please validate the PR.

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.

3 participants