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 upreplace err with badPrimitive #602
Conversation
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
eeue56
May 15, 2016
Contributor
@lukewestby does this solve any known issues? This seems legit, but it would be good to add a failing test case to the test suite first.
eeue56
commented
May 15, 2016
•
edited
Edited 1 time
-
eeue56
edited May 15, 2016 (most recent)
edited
-
May 15, 2016 (most recent)
|
@lukewestby does this solve any known issues? This seems legit, but it would be good to add a failing test case to the test suite first. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukewestby
May 15, 2016
Member
@eeue56 yep! See this thread from elm-dev. If you run the tests with this change they pass without the mentioned runtime error.
cc @rluiten
lukewestby
commented
May 15, 2016
•
edited
Edited 1 time
-
lukewestby
edited May 15, 2016 (most recent)
edited
-
May 15, 2016 (most recent)
|
@eeue56 yep! See this thread from elm-dev. If you run the tests with this change they pass without the mentioned runtime error. cc @rluiten |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
rluiten
May 15, 2016
Contributor
I have a test case that produces the failure.
Here it is, once luke suggested the fix, i figured it was expecting an object but got a list.
DecoderBug.elm.txt
Had to update the file i forgot to fix the expected string when it does not crash first time.
rluiten
commented
May 15, 2016
•
edited
Edited 1 time
-
rluiten
edited May 15, 2016 (most recent)
edited
-
May 15, 2016 (most recent)
|
I have a test case that produces the failure. DecoderBug.elm.txt |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
@lukewestby can you add the fail state to the test suite? thanks! |
added a commit
to rluiten/core
that referenced
this pull request
May 15, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukewestby
May 15, 2016
Member
I made a PR into @rluiten's fork with the test case, closing this one
|
I made a PR into @rluiten's fork with the test case, closing this one |
lukewestby commentedMay 14, 2016
•
edited
Edited 1 time
-
lukewestby
edited May 14, 2016 (most recent)
errisn't defined, it looks likebadPrimitiveis used elsewhere for the same purpose so perhaps it's just a typothanks!