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

A quick fix on https://github.com/elm-lang/core/issues/618 #619

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@ivanceras

ivanceras commented May 21, 2016

Making sure decoder and result is not undefined


This change is Reviewable

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 21, 2016

Contributor

This is not how this issue will be fixed.

Contributor

jvoigtlaender commented May 21, 2016

This is not how this issue will be fixed.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 21, 2016

Contributor

Well, let's leave that decision to Evan, actually. Maybe he wants to fix the Json decoder special case that way for the moment. But the general solution is of course to fix the compiler issue.

Contributor

jvoigtlaender commented May 21, 2016

Well, let's leave that decision to Evan, actually. Maybe he wants to fix the Json decoder special case that way for the moment. But the general solution is of course to fix the compiler issue.

@jvoigtlaender jvoigtlaender reopened this May 21, 2016

@ivanceras

This comment has been minimized.

Show comment
Hide comment
@ivanceras

ivanceras May 22, 2016

I should have used the word workaround. This is just a workaround, which solves the issue while a proper fix has not yet landed. This lets me continue work on my projects.

ivanceras commented May 22, 2016

I should have used the word workaround. This is just a workaround, which solves the issue while a proper fix has not yet landed. This lets me continue work on my projects.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender May 22, 2016

Contributor

Yes, I realize it's just a stop-gap measure for a specific case. Which is why I don't believe Evan is going to want to merge it into core.

As I commented in the issue itself, there are existing workarounds, without changing core. Mainly using a lazy-function.

Contributor

jvoigtlaender commented May 22, 2016

Yes, I realize it's just a stop-gap measure for a specific case. Which is why I don't believe Evan is going to want to merge it into core.

As I commented in the issue itself, there are existing workarounds, without changing core. Mainly using a lazy-function.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Sep 22, 2016

Member

It looks like this just makes the crash happen later. It will still definitely happen though as far as I can tell. Better to actually fix things.

Member

evancz commented Sep 22, 2016

It looks like this just makes the crash happen later. It will still definitely happen though as far as I can tell. Better to actually fix things.

@evancz evancz closed this Sep 22, 2016

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