Skip to content

ShareDB PR Review Meeting Notes 2018

Jump to bottom Edit New page
Eric Hwang edited this page Jul 25, 2018 · 2 revisions

2018-07-25

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 of getSnapshot, 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.
  • https://github.com/share/sharedb/pull/213 - Do not compose create with op (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 [email protected] branch started. Things that'd be in that branch so far:
    • Turning on by default the flag for not composing create with op 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.

2018-07-18

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.

PRs we looked at:

  • 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 to v
  • 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.

PRs we didn’t get to:

Action items:

2018-07-11

Attendees: Curran, Eric, Greg, Nate

PRs reviewed during meeting

We went through Greg's open PRs, as he was there to talk about them.

Next 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.

Add a custom footer
Add a custom sidebar
Clone this wiki locally