Skip to content
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

Patch changeListener to better sync self.current #13

Merged
merged 2 commits into from
Dec 20, 2014

Conversation

dashed
Copy link
Contributor

@dashed dashed commented Dec 17, 2014

Patch changeListener to better sync self.current when it's out of sync and when it's not.

  • needs tests
  • revised tests
  • review tests and implementation

Closes #12

@dashed dashed force-pushed the patch-changeListener branch from 6146b58 to 380e4a4 Compare December 17, 2014 23:12
@dashed
Copy link
Contributor Author

dashed commented Dec 17, 2014

rebased and squashed commits.

When a cursor updates, any cursors not in the keypath to the updated cursor will become stale. Patch changeListener to better sync self.current when a stale cursor updates, and when a un-stale cursor updates.
@dashed dashed force-pushed the patch-changeListener branch from 380e4a4 to 7f06ee7 Compare December 17, 2014 23:24
@dashed
Copy link
Contributor Author

dashed commented Dec 18, 2014

I'm currently revising tests.

@mikaelbr
Copy link
Member

Need to check this out, but immediately LGTM. The changes seem to make sense. I'll need to make a local copy and check it out with different test cases though. Maybe revise some tests.

@dashed
Copy link
Contributor Author

dashed commented Dec 18, 2014

I revised the tests. Feel free to change them for the better.

@mikaelbr
Copy link
Member

Good job. 👍 I'll look more into it later today, when I'm not so occupied and distracted.

dashed added a commit to dashed/immstruct that referenced this pull request Dec 18, 2014
Remove noop behaviour at original changeListener. This will be re-applied when omniscientjs#13 gets merged.
mikaelbr added a commit that referenced this pull request Dec 20, 2014
Patches changeListener to better sync self.current
@mikaelbr mikaelbr merged commit 7741732 into omniscientjs:master Dec 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revise how self.current syncs
2 participants