Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd support for large integers in Json decoder #352
Conversation
added some commits
Aug 16, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment
Hide comment
evancz
Aug 16, 2015
Member
Looks great to me, thank you! :D
@krisajenkins, does this have the behavior you were looking for?
|
Looks great to me, thank you! :D @krisajenkins, does this have the behavior you were looking for? |
pushed a commit
that referenced
this pull request
Aug 16, 2015
evancz
merged commit d1a0345
into
elm:master
Aug 16, 2015
1 check passed
continuous-integration/travis-ci/pr
The Travis CI build passed
Details
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment
Hide comment
krisajenkins
Aug 25, 2015
Contributor
Sorry for the delay in confirming - yes, this fix works for me. Thanks very much to both of you. :-)
|
Sorry for the delay in confirming - yes, this fix works for me. Thanks very much to both of you. :-) |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment
Hide comment
|
Great, thanks everyone! |
jvoigtlaender
referenced this pull request
Jan 19, 2016
Closed
Json.Decoder.int Bug / Ints in Elm #337
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ajhager commentedAug 16, 2015
The current Json int decoder only recognizes 32 bit integers. This PR provides a small test suite for the int decoder and a new decoder that works correctly with large numbers.
This benchmark (where 'original' is the old code and 'blend' is the code from this PR) shows (on OSX with Chrome at least) that the new code is slightly slower in the case that most decoded values are not ints, and nearly equivalent speed in the typical case when most values are ints.
ref #153 and #337