-
Notifications
You must be signed in to change notification settings - Fork 30
feat(json-crdt): Adds explanation about JSON-CRDT #35 #36
Conversation
@pgte what do you think about keeping this explanation under |
research/json-crdt.md
Outdated
|
||
An operation is a tuple of the form | ||
|
||
``` |
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.
Maybe using a screenshot of the paper where the tuple is represented would be easier to read. Since this is really informal, I believe we can just do it and loosely reference it?
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.
Yes, screenshot plus ref at the end is fine.
research/json-crdt.md
Outdated
|
||
### Applying Operations | ||
|
||
## Document editing API |
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.
Thinking of having a high level pseudo-code representation of the API here. Is it relevant?
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.
I think it is relevant, it will give a concrete sense of the semantics provided to the user.
research/json-crdt.md
Outdated
|
||
## Document editing API | ||
|
||
## Concurrent editing examples |
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 is a tricky part to add, but I really found helpful the way a similar section to this is in the paper. So I thought we could add those examples too. Would it be lame to just copy paste the examples? 😅
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.
You can add a markdown quote withe that text extracted from the paper.
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.
Will do, let's see how it works out!
research/json-crdt.md
Outdated
|
||
## Concurrent editing examples | ||
|
||
## Appendix |
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.
Do we need this or should we just reference to wikipedia/lamport clocks paper? This appendix is feeling a bit out of scope now.
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.
I think we can keep this informal, with direct links where needed, and no need for an appending IMO.
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.
Agreed
@gpestana yes, you can PR into the /research folder, it's a easier workflow if you want to debate things. |
@pgte I think this is shaping up now. Any comments/feedback on form and content? |
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 is great work, thank you for explaining this @gpestana!
research/json-crdt.md
Outdated
the cursor | ||
|
||
``` | ||
cursor(<MapT(doc), listT("shopping")>, id1) |
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.
What is the value of id1
here?
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.
- replace
id1
to something that makes sense given the context
research/json-crdt.md
Outdated
The `DELETE` and `ASSIGN` operations are destructive mutations. In the operation | ||
based JSON CRDT as presented in the paper, a deleted node is | ||
not removed from the document but instead it is represented by a node in which | ||
it `deps` set is empty. A document node which has its `deps` set empty is |
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.
Typo: should read “its deps
set”.
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.
- “its deps set”.
@gpestana merge at will! :) |
Awesome, thanks! I will fix those things and review it again and merge the PR. |
This PR is WIP
Opening this PR for starting to get feedback on form and content asap.