Fix for precision argument #5

Closed
wants to merge 3 commits into
from

2 participants

@pixelfreak

Here you go. This is for Issue #2. I added an argument called isNumInt because now prec is being used differently internally and externally.

I ran the test suite you have and the only thing that failed is the build#precision test, which makes sense because the behavior has changed. The test is updated with the isNumInt set to true.

Please review. Thanks!

@defunctzombie

Not sure I like adding an isNumInt argument, I feel like the old (from int) api can maybe go away.

Also, when making patches, please follow the coding style of the project :)

I will review the changes and think of the isNumInt argument. My initial thoughts are that it isn't needed and we can just stick with the precision.

@pixelfreak

What do you mean by stick with the precision?

@defunctzombie
@pixelfreak

I see. Well, let me know when you have decided. Meanwhile, I'll go with what I have for now. Thanks!

@defunctzombie

214d0b4

So it seems the main reason the precision/int combination exists is for internal use. I am thinking that a separate method could exist num.fromInt or some such that would have a decimal offset field.

@pixelfreak

Sweet! Works for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment