Closed pvh closed 3 years ago
Nice work. Still going through it carefully but one initial piece of feedback that stood out:
Some of the code examples reference variables from a global scope that aren't explained. For example, the updatePeers
sample code references syncStates
, but its never explicitly explained what the shape and contents of that variable. A newcomer would have to trace the code carefully to figure out it was a map of peerIds to syncStates.
For any variables like that, I suggest including them above the function, with maybe an example implementation.
In the example code, you're showing multiple docs being tracked and synced, with backends
as a map of docId to backend. Would it be simpler for newbies to have these examples just show how 1 document would be synced? The backends
map adds another level of complexity to grok, as opposed to a simpler example of just syncing a single backend.
In the example code, you're showing multiple docs being tracked and synced, with
backends
as a map of docId to backend. Would it be simpler for newbies to have these examples just show how 1 document would be synced? Thebackends
map adds another level of complexity to grok, as opposed to a simpler example of just syncing a single backend.
I thought about that and decided given that everyone I've ever seen using automerge has >1 document it would be actually more confusing if I forced people to think about how to generalize to >1 document instead of just showing them but maybe that's wrong?
Thanks for the comments. I think I've addressed them all.
First sentence:
exchanging messages between peers until both documents [missing word] they have the same contents
missing word = "think," "know," "believe," "hope," ...
"infer", "pray", "imagine"...
Thanks.
On Fri, May 21, 2021 at 4:13 PM Steve Schafer @.***> wrote:
First sentence:
exchanging messages between peers until both documents [missing word] they have the same contents
missing word = "think," "know," "believe," "hope," ...
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/automerge/automerge/pull/383#issuecomment-846304690, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWQB2ZHDSTT2M4XLKHIDTO3SKNANCNFSM45FG6ABQ .
-- Peter van Hardenberg "Everything was beautiful, and nothing hurt."—Kurt Vonnegut
Very nice work, @pvh. I've added a commit with some light editing.
Your examples work directly with the Backend APIs, but I suspect most users will be using the top-level Automerge object functions, not separating frontend and backend. What do you think about changing the examples to use the top-level functions instead? The frontend/backend split is pretty much orthogonal to the sync protocol, and it deserves its own document.
Your examples work directly with the Backend APIs, but I suspect most users will be using the top-level Automerge object functions, not separating frontend and backend. What do you think about changing the examples to use the top-level functions instead? The frontend/backend split is pretty much orthogonal to the sync protocol, and it deserves its own document.
An up-to-date description of how the frontend and backend work together would be very appreciated (at least by me). I would expect to find it either in INTERNALS.md or in a separate document, not in SYNC.md.
This is on my TODO list, but having recently worked on the sync APIs I had this top of mind and so wrote it first.
Okay, thanks all for your contributions. I will merge this, and we can always update the document on subsequent PRs.
Here's a PR for the sync protocol docs. I've included some example code from automerge-demo and some explanation. This documentation would probably benefit from a few figures/diagrams and maybe a couple of examples but I think it's a good start.
I'd particularly like to hear from @skokenes, @okdistribute, and @HerbCaudill or anyone else who has attempted to implement the sync protocol if this would have covered your questions and set you on the right path.