Clarify the rules for deriving the delta base payload #356
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I found the existing rules quite hard to understand; in particular it was not very clear how to derive the base payload in the case where you have a non-delta, non-Base64 message.
lawrence-forooghian
added a commit
to ably/ably-cocoa
that referenced
this pull request
Aug 12, 2025
Fix the following scenarios: - delta decoding of JSON-valued message data: we were not setting the base payload to the message's `data` before decoding JSON - delta decoding when using the non-binary protocol: we were setting the base payload to the delta, not the result of vcdiff decoding Specification references based on [1] at adf8d0f. Resolves #2082. [1] ably/specification#356
lawrence-forooghian
requested review from
SimonWoolf,
VeskeR,
maratal,
sacOO7,
ttypic and
umair-ably
SimonWoolf
reviewed
Aug 15, 2025
| *** @(RTL19e2)@ Else, the message's @data@ is a delta, and the base payload is the result of using the @vcdiff@ decoder to apply this delta to the stored base payload. | ||
| ** @(RTL19a)@ This clause has been deleted. It was valid up to and including specification version @TBD@. | ||
| ** @(RTL19b)@ This clause has been deleted. It was valid up to and including specification version @TBD@. | ||
| ** @(RTL19c)@ This clause has been deleted. It was valid up to and including specification version @TBD@. |
There was a problem hiding this comment.
Since you're just rewording for clarity (and adding some sub-items) and aren't actually changing what it means, you don't need to deprecate the old spec items - you could just keep RTL19d as RTL19b and RTL19e as RTL19c. (otherwise you're forcing any sdk devs who referenced the old spec items in comments to go and update them to the new ones, even though there's no actual substantive change)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found the existing rules quite hard to understand; in particular it was not very clear how to derive the base payload in the case where you have a non-delta, non-Base64 message.