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

[failing] add tests for equality of Json.Encode.Value #825

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@soenkehahn

soenkehahn commented Jan 28, 2017

Equality (elm's ==) is not commutative for Json.Encode.Values created with object. I believe this is a bug. This PR adds failing test cases but not a fix.

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Jan 28, 2017

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Jan 28, 2017

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Mar 7, 2018

Member

To restrict equality to only work on sensible values, it requires changes in the compiler. Those changes are pretty involved, and the problem is known. Overall, I do not think having a failing test will bring us closer to the fix, and it will make other things harder in the meantime.

Member

evancz commented Mar 7, 2018

To restrict equality to only work on sensible values, it requires changes in the compiler. Those changes are pretty involved, and the problem is known. Overall, I do not think having a failing test will bring us closer to the fix, and it will make other things harder in the meantime.

@evancz evancz closed this Mar 7, 2018

@soenkehahn

This comment has been minimized.

Show comment
Hide comment
@soenkehahn

soenkehahn Mar 7, 2018

Sorry, this was not meant to be merged as is, only to provide a very concrete bug report. (Therefore it's completely fair to close this PR.)

Those changes are pretty involved, and the problem is known.

Is this problem tracked somewhere? Could you point me to it, please?

soenkehahn commented Mar 7, 2018

Sorry, this was not meant to be merged as is, only to provide a very concrete bug report. (Therefore it's completely fair to close this PR.)

Those changes are pretty involved, and the problem is known.

Is this problem tracked somewhere? Could you point me to it, please?

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Mar 7, 2018

Member

There's not a public issue. The path is not "4 clear steps" that just have to be implemented by anyone, so I don't think having it tracked on GitHub really helps fix things. Design choices need to be made, making those choices needs to be high priority, they need to fit with a release, etc. So it's at a point where I feel that public comment does not help with any specific piece of work, and accepting public comment may make things slower and take people's time for no particular reason.

Member

evancz commented Mar 7, 2018

There's not a public issue. The path is not "4 clear steps" that just have to be implemented by anyone, so I don't think having it tracked on GitHub really helps fix things. Design choices need to be made, making those choices needs to be high priority, they need to fit with a release, etc. So it's at a point where I feel that public comment does not help with any specific piece of work, and accepting public comment may make things slower and take people's time for no particular reason.

@soenkehahn

This comment has been minimized.

Show comment
Hide comment
@soenkehahn

soenkehahn Mar 8, 2018

Sounds good, thanks!

soenkehahn commented Mar 8, 2018

Sounds good, thanks!

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