Conversation
WalkthroughAdds nested operation payloads for protocol v6+ by extending the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lmars
left a comment
There was a problem hiding this comment.
That being said, also happy to merge this as is.
Extend the Operation struct with fields for action-specific payloads used by protocol v6: MapCreate, MapSet, MapRemove, CounterCreate, CounterInc, and ObjectDelete. Define corresponding V2 structs that carry the per-action data (semantics, entries, keys, counts, etc.).
0441f0d to
0cf066d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@ably/objects/liveobjects.go`:
- Around line 160-166: The JSON/codec struct tags on CounterCreate.Count and
CounterInc.Number include "omitempty", which will omit valid zero float64 values
(e.g., CounterCreate{Count: 0} or CounterInc{Number: 0}) from the wire; remove
"omitempty" from both the `json` and `codec` tags for the Count field in type
CounterCreate and the Number field in type CounterInc so zero values are encoded
and preserved for v6+ types.
- Around line 146-149: The Semantics field in MapCreate (and the similar Map
struct) uses `omitempty` so the valid zero-value semantic Map_LWW (value 0) is
omitted during serialization; remove `omitempty` from the JSON and codec tags
for the Semantics field (e.g., in struct MapCreate.Semantics and Map.Semantics)
so the semantics value is always emitted even when zero, preventing silent drops
and server-side misinterpretation.
🧹 Nitpick comments (2)
ably/objects/liveobjects.go (2)
151-154: Naming inconsistency:MapSet.ValuevsMapOp.Data.The existing
MapOpusesData *Datawhile the newMapSetusesValue *Data. If both carry the same semantic (the data/value for a map key), consider aligning the naming for consistency — or document the rationale for the difference if it's intentional per the v6 protocol spec.
168-168: EmptyObjectDeletestruct — consider adding a brief doc comment.An empty struct with no fields and no comment may confuse future readers about whether fields were accidentally omitted. A short doc comment clarifying it's intentionally empty (the action itself is the signal, no payload needed) would help.
Extend the Operation struct with fields for action-specific payloads used by protocol v6: MapCreate, MapSet, MapRemove, CounterCreate, CounterInc, and ObjectDelete.
Define corresponding V2 structs that carry the per-action data (semantics, entries, keys, counts, etc.).
Summary by CodeRabbit