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

Move Client.Watch inside Client.Attach and hide it from external interface #584

Open
hackerwins opened this issue Jul 19, 2023 · 5 comments
Labels
cleanup 🧹 Paying off technical debt enhancement 🌟 New feature or request sdk ⚒️

Comments

@hackerwins
Copy link
Member

Description:

Move Client.Watch inside Client.Attach and hide it from the external interface.

Go SDK is just used in integration tests of servers without other SDK installations. So it was OK to expose Client.Watch to the external interface. But by adding more and more features to the SDK, it is quite difficult to keep simple tests.

Let's move Client.Watch inside Client.Attach and hide it from the external interface to maintain consistency with other SDKs and simplify testing.

Why:

Keep the product simple

@hackerwins hackerwins added enhancement 🌟 New feature or request good first issue 🐤 Good for newcomers cleanup 🧹 Paying off technical debt labels Jul 19, 2023
@karockai
Copy link
Contributor

It looks interesting. May I work on this issue?

@krapie
Copy link
Member

krapie commented Jul 22, 2023

@karockai sure! give it a try!

@krapie
Copy link
Member

krapie commented Feb 27, 2024

Do we need to change the entire interface to perform runWatchLoop() and provide subscribe() interface just like other SDKs?

@chacha912
Copy link
Contributor

@krapie Yes, that seems necessary.
Here's the discussion we had about what tasks need to be carried out regarding this issue, for your reference.
https://codepair.yorkie.dev/community/65e02d3166a65e76f6703d15/share?token=2ij9re

@hackerwins
Copy link
Member Author

Following #803, we need to apply the below features:

  • Reconnection logic for watch connection failure.
  • Apply SyncMode introduced in the JS SDK.

yorkie-team/yorkie-js-sdk#772

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt enhancement 🌟 New feature or request sdk ⚒️
Projects
Status: Backlog
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants