Skip to content
This repository was archived by the owner on Jan 7, 2025. It is now read-only.

Added new fields: action (TM2j) and serial (TM2k).#43

Open
maratal wants to merge 1 commit intomainfrom
message-action-and-serial
Open

Added new fields: action (TM2j) and serial (TM2k).#43
maratal wants to merge 1 commit intomainfrom
message-action-and-serial

Conversation

@maratal
Copy link
Copy Markdown
Collaborator

@maratal maratal commented Nov 25, 2024

Part of a bigger task (ably/ably-cocoa#2000).


## enum MessageAction

Describes the possible actions message can represent (in order from zero).
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.

Where did these descriptions come from? It's not very clear to me from looking at these how a user of the library is meant to use these actions or the two new Message fields.

| name: String? ||| TM2g | The event name. |
| timestamp: Time ||| TM2f | Timestamp of when the message was received by Ably, as milliseconds since the Unix epoch. |
| action: MessageAction ||| TM2j | The [`MessageAction`]{@link MessageAction} this message represents. |
| serial: String? ||| TM2k | An opaque string that uniquely identifies the message. |
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.

It's not optional in the IDL. Also, how is this different to id?

@lawrence-forooghian
Copy link
Copy Markdown
Collaborator

This stuff should have had docstrings added here when it was added to the spec. Somebody has added them in JS already, it seems. https://github.com/ably/ably-js/blob/18a255948c38d1e60715c8f5d6173369b57cb8d6/ably.d.ts#L2337-L2429. Could you find out who did this work (because they probably are much better positioned to write meaningful docstrings for these changes, which mean nothing to me) and ask them to add the docstrings to this repo?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants