Make Faker::Number return integers and floats instead of strings #510
Conversation
|
This would require a version increase to 2.x for breaking changes, I think |
|
Yeah, I think a version bump is good idea. I'll make that happen when I merge this. |
|
Hi guys. |
tejasbubane
force-pushed
the
number-values-not-strings
branch
from
94f78b7 to
38fa623
Compare
|
@ivanovaleksey Rebased |
|
Thank you @tejasbubane. I think now it can be merged? |
|
Agreed on putting this in 2.0. |
|
@stympy Will it be merged? Maybe this could also go into 1.9? |
vbrazo
force-pushed
the
master
branch
5 times, most recently
from
a359def to
a5d7731
Compare
tejasbubane
force-pushed
the
number-values-not-strings
branch
3 times, most recently
from
e676509 to
b4e8d17
Compare
|
@vbrazo I have rebased the PR and fixed merge conflicts. |
|
@vbrazo is anyone is working on this PR, seems there is conflict in this PR. |
|
@gkunwar yes @tejasbubane is on it. I think we could create a @tejasbubane could you rebase with |
tejasbubane
force-pushed
the
number-values-not-strings
branch
3 times, most recently
from
776a284 to
0e062f6
Compare
* Change `Number.number` to return integers. * Remove `leading_zero_number` because it has no meaning now that we are returning integers. * Remove `decimal_part` because it can now be replaced by `number`. * Fix all failing tests
tejasbubane
force-pushed
the
number-values-not-strings
branch
from
0e062f6 to
3b2df30
Compare
|
@vbrazo I have rebased on |
|
|
||
| # Required parameter: digits | ||
| # Produces a number of the specified digits with a leading zero | ||
| Faker::Number.leading_zero_number(10) #=> "0669336915" |
There was a problem hiding this comment.
I have removed leading_zero_number because it has no meaning now that we are returning integers.
|
|
||
| # Required parameter: digits | ||
| # Produces a 2-digit number, preserves leading 0's | ||
| Faker::Number.decimal_part(2) #=> "09" |
There was a problem hiding this comment.
I removed decimal_part because it can now be replaced by number/number.to_s as required.
|
if you want to remove the methods, you need to deprecate them first. |
|
@vbrazo This PR is against |
|
Please open a new PR and deprecate the |
|
Hi, despite this being recognised as a breaking change, it wasn't included in the main breaking changes list for the v2 release. Could this be fixed, just to save some other devs like me headache in the future? |
…er-ruby#510) * Change `Number.number` to return integers. * Remove `leading_zero_number` because it has no meaning now that we are returning integers. * Remove `decimal_part` because it can now be replaced by `number`. * Fix all failing tests
…er-ruby#510) * Change `Number.number` to return integers. * Remove `leading_zero_number` because it has no meaning now that we are returning integers. * Remove `decimal_part` because it can now be replaced by `number`. * Fix all failing tests
Fixes #237 #1651
I think
Faker::Numbershould return integers and floats instead of strings.