Skip to content

rts: Document the ops argument of allowUpdate where it is declared - #4216

Open
marksvc wants to merge 1 commit into
masterfrom
task/sec1-1
Open

marksvc wants to merge 1 commit into
masterfrom
task/sec1-1

Conversation

@marksvc

@marksvc marksvc commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

The note about what an update rule receives sat on allowUpdate itself, where
an editor shows it when hovering the method rather than the argument it
describes. It now sits on the argument, which is what a rule author is looking
at when they need it.

The wording also claimed the ops are the op as the client sent it, which is not
so: ShareDB transforms a client's op against any ops that the client did not
have before the update rules run, so paths, values and metadata can all differ
from what was sent.


Open in Devin Review


Stack created with GitHub Stacks CLI • Give Feedback 💬


This change is Reviewable

@marksvc
marksvc added this pull request to stack #4226 October 22, 2026 21:54
@marksvc
marksvc deployed to screenshot_diff October 22, 2026 22:01 — with GitHub Actions Active
@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.20%. Comparing base (f3a318d) to head (a51915b).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4216   +/-   ##
=======================================
  Coverage   81.20%   81.20%           
=======================================
  Files         670      670           
  Lines       42913    42913           
  Branches     7111     7111           
=======================================
  Hits        34846    34846           
  Misses       6902     6902           
  Partials     1165     1165           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@marksvc marksvc added testing not required ai-review-author-sign-off PR author has looked at AI review of change and determined no findings warrant further changes labels Oct 22, 2026
@marksvc
marksvc marked this pull request as ready for review October 22, 2026 22:48
@marksvc
marksvc deployed to screenshot_diff October 23, 2026 16:10 — with GitHub Actions Active
@marksvc
marksvc removed this pull request from stack #4226 October 23, 2026 16:11
@marksvc
marksvc added this pull request to stack #4228 October 23, 2026 16:11

@pmachapman pmachapman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@pmachapman reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on marksvc).

@pmachapman pmachapman self-assigned this Oct 27, 2026
The note about what an update rule receives sat on allowUpdate itself, where
an editor shows it when hovering the method rather than the argument it
describes. It now sits on the argument, which is what a rule author is looking
at when they need it.

The wording also claimed the ops are the op as the client sent it, which is not
so: ShareDB transforms a client's op against any ops that the client did not
have before the update rules run, so paths, values and metadata can all differ
from what was sent.

This branch was successfully deployed

1 active deployment
screenshot_diff — a51915b0 Deployed Oct 27, 2026 by pmachapman via Compare Screenshots #1219
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review-author-sign-off PR author has looked at AI review of change and determined no findings warrant further changes testing not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants