/core

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

Fix Dict.merge problem where some elements ignored from second dict #648

Merged
merged 1 commit into from Jun 25, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@fredcy @knewter @evancz
@fredcy
Contributor

fredcy commented Jun 17, 2016

I added tests that demonstrate the problem. In particular, the "merge singletons in order" and "partially overlapping" tests will fail without the changes to Dict.merge.
@fredcy

This comment has been minimized.

Show comment
Hide comment
@fredcy

fredcy Jun 17, 2016

Contributor

@knewter: This addresses that issue you raised.
Contributor

fredcy commented Jun 17, 2016

@knewter: This addresses that issue you raised.
@knewter

This comment has been minimized.

Show comment
Hide comment
@knewter

knewter Jun 17, 2016

(for reference, the issue @fredcy is mentioning is here: https://groups.google.com/forum/#!topic/elm-dev/gbYNWTT0TsM)

knewter commented Jun 17, 2016

(for reference, the issue @fredcy is mentioning is here: https://groups.google.com/forum/#!topic/elm-dev/gbYNWTT0TsM)

knewter referenced this pull request in knewter/elm-phoenix-socket Jun 17, 2016

@knewter
Use updated merge function 
I identified [a
bug](https://groups.google.com/forum/#!topic/elm-dev/gbYNWTT0TsM) in
Elm's `Dict.merge` function, and [@fredcy fixed
it](https://github.com/elm-lang/core/pull/648)  It's not likely to be
merged / available in the short term, so I'm using that function here
instead of core's `Dict.merge` now, and updated my tests accordingly.
67fa567
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jun 25, 2016

Member

Cool, looks good to me @fredcy. Thanks!
Member

evancz commented Jun 25, 2016

Cool, looks good to me @fredcy. Thanks!

@evancz evancz merged commit 913f800 into elm:master Jun 25, 2016
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@fredcy fredcy deleted the fredcy:dict-merge branch Jun 26, 2016

knewter referenced this pull request in knewter/elm-phoenix-socket Jul 17, 2016

@knewter
Add Presence Support 
- Add tests, initial syncState, syncDiff.
- Use an updated dict merge function, because of this bug in Elm's
  Dict.Merge: https://github.com/elm-lang/core/pull/648
- Parameterize user payloads
- Add documentation for Presence
6c90453

@knewter knewter referenced this pull request Jul 17, 2016

Closed

Add Presence Support #16

maxstr referenced this pull request in fbonetti/elm-phoenix-socket Mar 20, 2017

@knewter @maxstr
Add Presence Support 
- Add tests, initial syncState, syncDiff.
- Use an updated dict merge function, because of this bug in Elm's
  Dict.Merge: https://github.com/elm-lang/core/pull/648
- Parameterize user payloads
- Add documentation for Presence
201f6c1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment