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

Sync presence and undo redo #229

Open
wants to merge 56 commits into
base: master
Choose a base branch
from

Conversation

gkubisa
Copy link
Contributor

@gkubisa gkubisa commented Jul 20, 2018

Combines #216 and #207 and resolves all conflicts.

The problem was that unsubscribe re-added the doc to the connection.
Now the doc is removed from the connection after unsubscribe.
Additionally, we're no longer waiting for the unsubscribe response
before executing the callback. It is consistent with Query,
unsubscribe can't fail anyway and the subscribed state is updated
synchronously on the client side.
I had to add the --exit flag workaround to mocha.opts to make it exit
when tests are done. A better long-term solution would be to ensure
that nothing keeps node running after all tests are done, see
https://boneskull.com/mocha-v4-nears-release/#mochawontforceexit.
The problem was that unsubscribe re-added the doc to the connection.
Now the doc is removed from the connection after unsubscribe.
Additionally, we're no longer waiting for the unsubscribe response
before executing the callback. It is consistent with Query,
unsubscribe can't fail anyway and the subscribed state is updated
synchronously on the client side.
The issue could not cause problems in practice because
ot-json0 does not support presence.
@coveralls
Copy link

coveralls commented Jul 20, 2018

Coverage Status

Coverage increased (+0.4%) to 96.916% when pulling 543307f on Teamwork:sync-presence-and-undo-redo into 762e05d on share:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 96.916% when pulling 543307f on Teamwork:sync-presence-and-undo-redo into 762e05d on share:master.

@curran
Copy link
Contributor

curran commented Jul 22, 2020

Suggest to close as stale.

@curran curran mentioned this pull request Oct 21, 2020
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.

3 participants