add clipped flag to annotation summary spec #382
Merged
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
AndyTWF
reviewed
Sep 22, 2025
textile/api-docstrings.md
Outdated
| | total | number | The sum of the counts from all clients who have published an annotation with this name. | | ||
| | clientIds | { [key: string]: number } | A list of the clientIds of all clients who have published an annotation with this name, and the count each of them have contributed. | | ||
| | totalUnidentified | number | The sum of the counts from annotations of unidentified clients. | | ||
| | clipped | boolean | Indicates whether the clientIds map has been clipped (the total and totalClients counts are unaffected). | |
Contributor
There was a problem hiding this comment.
Perhaps be more specific and say that total/totalClientIds/totalUnidentified represent the values of the full-unclipped summary?
Contributor
Author
There was a problem hiding this comment.
Suggested change
| | clipped | boolean | Indicates whether the clientIds map has been clipped (the total and totalClients counts are unaffected). | | |
| | clipped | boolean | Indicates whether the clientIds map has been clipped. Note that `total` and `totalClients` represent the values of the unclipped summary. | |
?
textile/features.textile
Outdated
| **** @(TM7d1a)@ @total@ integer | ||
| **** @(TM7d1b)@ @clientIds@ @Dict<string, integer>@ | ||
| **** @(TM7d1c)@ @clipped@ boolean (must be forced to false if not set over the wire) | ||
| **** @(TM7d1d)@ @totalUnidentified@ integer |
Contributor
There was a problem hiding this comment.
We should document these properties with what they do, as we do in TM2*.
textile/features.textile
Outdated
| *** @(TM7c1)@ @SummaryClientIdList@ is an object containing: | ||
| **** @(TM7c1a)@ @total@ integer | ||
| **** @(TM7c1b)@ @clientIds@ array of strings | ||
| **** @(TM7c1c)@ @clipped@ boolean (must be forced to false if not set over the wire) |
Contributor
There was a problem hiding this comment.
Nitpick for consistency - a similar wording to TM2*, in form of "If not set on the wire, the SDK must ... "
Contributor
Author
|
Updated. @AndyTWF I've pushed a new commit but I'll squash them into one before merging. |
Contributor
|
@vladvelici Looks like https://github.com/ably/specification/pull/382/files#r2368172590 still needs a conclusion? |
Co-authored-by: Andy Ford <[email protected]>
Contributor
Author
|
@AndyTWF sorry missed that one, done |
AndyTWF
approved these changes
Sep 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Added
clipped,totalClientIdandtotalUnidentifiedwhere they were missing.