-
Notifications
You must be signed in to change notification settings - Fork 5
(feat): Sync Streams #35
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
Conversation
…ler logic to StreamingSyncImplementation.
simolus3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look at sync streams 🚀 I have a few high-level / API design comments from a quick look, but please treat them as optional since I'm not that familiar with the dotnet SDK.
…ia a factory. Scaffolding A MockRemote test.
simolus3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me from a high-level perspective (since I don't know C# that much).
LucDeCaf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy overall, might be able to improve the user-facing API a little bit.
361396c to
9d9bd22
Compare
Added support for sync streams, based on reference implementations from other SDKs.
Minor changes were made to the API to support mocking of the remote, this allows new possibilities with mocking the backend when testing.