-
Notifications
You must be signed in to change notification settings - Fork 635
Add NewFromInt #72
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
Add NewFromInt #72
Conversation
First, thanks for the additions! However, is there a reason you chose Why not If there are precedents elsewhere for this kind of thing elsewhere I could be convinced that it's a good idea, but on my first glance it seems rather confusing to have Naming aside, this addition looks great and I'm happy to have more "newFroms" in Decimal! |
Just following up here, any reply to the above @mathieupost ? |
Sorry for the late reply, I'm on a holiday at the moment ;) |
@victorquinn any comment on this? |
I agree that |
Since NewFromFloat uses float64 and most libraries use int64 for integers I'll just keep the NewFromInt(int64) method and remove the NewFromInt32(int32) method, since then it'll be consistent with the rest of the API of this library. |
Looks like you included your |
Oops, fixed ;) |
please merge it |
Hi @mathieupost, a new maintainer here. Thanks for your contribution. |
Hi! This is a long time ago 😅 I guess the idea was that with the |
I think your point is pretty valid. |
I could add |
Use NewFromInt in example in the README Change to big.NewInt() Add TestNewFromInt and remove NewFromInt32 Fix test Add NewFromInt32 again
IMO, looks legit. : ) Let's wait for Jason's feedback |
Add NewFromInt and NewFromInt32
Resolves #11