-
Notifications
You must be signed in to change notification settings - Fork 41
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
[DTP-732] - Create LiveSync Quickstart page #2184
Conversation
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.
Hi @GregHolmes , I've added a few comments wrt the structure of the quickstart guide. Worth looking into those and I can do a fresh round of review.
d66e696
to
19c8c89
Compare
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 Greg - as requested by @Srushtika the example has now been moved to its own repo: https://github.com/ably-labs/live-comments
So some changes required to align with that. I also changed the implementation of how we subscribe to the model to do it the same was as we do in the livesync
example which handles model subscribe/unsubscribe better
c2f3c71
to
599bdd8
Compare
Thanks @mschristensen I've updated these. |
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.
Looks good, minor comment on some unused env vars, once addressed this looks good. Thanks Greg
2e1a860
to
e461b24
Compare
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 @GregHolmes - I've added some comments
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 the changes @GregHolmes , added a few minor comments, hopefully the last round.
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.
Nice work, thanks @GregHolmes
86e30f1
to
0bfea92
Compare
Description
This PR is a further improvement to the LiveSync docs for the Alpha release of the product. This contains a Quickstart page.