-
Notifications
You must be signed in to change notification settings - Fork 452
ShareDB PR Review Meeting Notes 2018
Eric Hwang edited this page Jul 25, 2018
·
2 revisions
Attendees: Alec, Eric, Nate, Felipe (Alec's co-worker)
PRs we discussed:
-
https://github.com/share/sharedb/pull/230 - Remove sharedb-mingo-memory circular dependency (Eric)
- sharedb had depended on sharedb-mingo-memory for 4 tests, mostly relating to query subscriptions. The PR adds a minimal fake of query filtering/sorting to the tests, which removes the need for the dev dependency on sharedb-mingo-memory.
- PR is merged, will get picked up in the next publish.
-
https://github.com/share/sharedb/pull/220 - Add methods to fetch snapshots and snapshots at time (Alec)
- Decided to name the method
fetchSnapshot
instead ofgetSnapshot
, to indicate it's asynchronous. - As there’s still some debate in the PR comments about how to implement the fetch-snapshot-at-time method performantly, Alec will take it out of the PR for now.
- Nate says fetch-snapshot-at-time might need some supporting functionality in sharedb-mongo, like optionally being able to add indexes for efficient timestamp querying of ops. We discussed it for a bit, will likely take more discussion later.
- Decided to name the method
-
https://github.com/share/sharedb/pull/213 - Do not compose
create
withop
(Greg)- Nate realized that this is technically a breaking change that may affect some kinds of middleware.
- Nate will add an off-by-default flag for this do-not-compose behavior before merging the PR.
- In the upcoming 1.0.0 branch, Nate will make the flag on by default and document the breaking change.
Some discussion unrelated to specific Share PRs:
- Alec wants to add some things to json0, can anyone merge and publish it?
- Nate can merge, but he can’t publish.
- Nate will see if he can reach Joseph to get publishing permissions on json0.
- Alec: Ideas on handling end-user-accessible metadata for rich text documents?
- We decided that creating a wrapper type would probably be the best way forward.
- Nate: I'd like to get a sharedb@1.0.0 branch started. Things that'd be in that branch so far:
- Turning on by default the flag for not composing
create
withop
above. - Switching ShareDB’s error codes from error code numbers to error code strings, to match Node’s error format. Originally, it was numbers because Mongo and websockets use numbers, but strings are clearer.
- Turning on by default the flag for not composing
Attendees: Alec, Curran (second half), Eric, Greg, Nate
In the past week, Nate has been mostly focused on finishing out pending work in Derby, which will be done this week. He did merge in the WebSocket reconnection PR (which one?), and he switched the examples over to the Teamwork version of websocket server.
-
https://github.com/share/sharedb/pull/220 (getSnapshotAtVersion - Alec)
- If you’re going to get an older version of a snapshot, you’re going to either rewind ops from the current snapshot with inverse, or start with the original create and replay the snapshots on top.
- How do we handle revert to a previous version cleanly? You could do a diff-match-patch with the target version like Git does, or you could invert the ops from between the target version or now.
- We don’t want to back ourselves into a corner API design wise, by returning just the snapshot data and just letting the user use it as they want.
- submitSnapshot from the undo/redo PR uses diff/diffX on the OT type to submit the snapshot
- Action items:
- Add a base Snapshot class, have MemorySnapshot and eventually MongoSnapshot inhert from it
- Return an actual snapshot object, i.e. change
version
tov
-
https://github.com/share/sharedb/pull/224 (customizing clientId - Alec)
- Goal is to attribute ops to specific users
- Customizing is a good idea
- If we allow customizing, we should allow customizing the clientId entirely
- Putting custom clientId as a prefix means it can be queried efficiently with Mongo
- Eric: We use server-side ShareDB middleware to store attribution info for an op on the op meta. Downside is that you don’t get that info in the client, which in our case is intentional.
- Nate: We can certainly add an option to return parts of the op meta to the client, with a customizable projection.
- Alec has decided to close his PR and put the info on the op meta.
-
https://github.com/share/sharedb-mongo/pull/58 (improve perf of getLinkedOps - Alec)
- Hooray performance improvement! Looks good to merge.
- We’ll want to fix up the tests first, though. Greg has two PRs against sharedb-mingo-memory and sharedb-mongo to fix them. Eric will take care of handling the merge and publish of them.
-
https://github.com/share/sharedb/pull/216 (undo/redo - Greg)
- Making good progress
- Brief discussion around naming of undoComposeTimeout, which controls how close a new op is to previous ones for the undo manager to compose them. Decided to rename to undoComposeInterval.
- https://github.com/share/sharedb/pull/207 - Presence
- Eric: Merge and publish https://github.com/share/sharedb-mingo-memory/pull/5 (done, published as sharedb-mingo-memory@1.0.1)
- Greg: Update https://github.com/share/sharedb-mongo/pull/59 to use the published sharedb-mingo-memory version, assign PR to Eric for merging and publishing
- Nate: Merge and publish a couple sharedb PRs from last week, or grant publishing permissions to Eric and delegate to him:
- Nate: Take a last look at these PRs, then merge+publish them:
- Eric to investigate putting multiple NPM repos into one GitHub repo, to see if it’s feasible to combine sharedb-mingo-memory into sharedb-mongo.
- Greg and Alec to continue work on their PRs
- Eric to add Greg, Curran, and Alec to the recurring calendar invite. We will probably still post the meeting link publicly, in case others want to join.
Attendees: Curran, Eric, Greg, Nate
We went through Greg's open PRs, as he was there to talk about them.
- Ready to merge - @nateps will take care of merging and publishing in the next day or two:
- Should be good to go, but before merging, @nateps is going to read through Share code related to the PRs as a sanity check:
- @gkubisa to do some more updates on these, based on discussions:
- https://github.com/share/sharedb/pull/204 - Small tweak, doable quickly
- https://github.com/share/sharedb/pull/216 - Will need some more experimenting with doing undo/redo stack across multiple docs
- Deferring until next week's meeting:
- https://github.com/share/sharedb/pull/207 - We felt Presence would probably take more time to discuss, so we prioritized the other PRs this week.
Review Presence and the other PRs we didn't get to this week. If we have time, we'll look at PRs in the other repos like sharedb-mongo.