Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chat: add CHA-RL9 to explain "attaching piggyback" #232

Merged
merged 1 commit into from
Nov 25, 2024
Merged

chat: add CHA-RL9 to explain "attaching piggyback" #232

merged 1 commit into from
Nov 25, 2024

Conversation

AndyTWF
Copy link
Contributor

@AndyTWF AndyTWF commented Nov 19, 2024

Closes #228

@AndyTWF AndyTWF requested review from lawrence-forooghian and a team November 19, 2024 18:28
@lawrence-forooghian lawrence-forooghian mentioned this pull request Nov 20, 2024
Closed
@AndyTWF AndyTWF requested review from a team, owenpearson and JoaoDiasAbly and removed request for a team November 21, 2024 13:48
lawrence-forooghian added a commit to ably/ably-chat-swift that referenced this pull request Nov 21, 2024
@lawrence-forooghian
Implement CHA-RL9
447b72d 
Based on [1] at 32b642c.

Resolves #131.

[1] ably/specification#232
@lawrence-forooghian lawrence-forooghian mentioned this pull request Nov 21, 2024
Merged
lawrence-forooghian added a commit to ably/ably-chat-swift that referenced this pull request Nov 21, 2024
@lawrence-forooghian
Implement CHA-RL9
40958c3 
Based on [1] at 32b642c.

Resolves #131.

[1] ably/specification#232
@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 22, 2024

This is also applicable on sending ephermal reactions via send method right?

@AndyTWF
Copy link
Contributor Author

AndyTWF commented Nov 22, 2024

This is also applicable on sending ephermal reactions via send method right?

Yes, but I decided not to alter the spec here because there's currently a DR open that will make that spec point entirely redundant.

@sacOO7 sacOO7 mentioned this pull request Nov 22, 2024
Closed
sacOO7
sacOO7 approved these changes Nov 22, 2024
View reviewed changes
Copy link
Collaborator

@sacOO7 sacOO7 left a comment

Choose a reason for hiding this comment

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

LGTM

@lawrence-forooghian
Copy link
Collaborator

I want to get this one merged so that I can implement it in Swift; have rebased and will merge.

@lawrence-forooghian lawrence-forooghian merged commit bc46003 into main Nov 25, 2024
2 checks passed
@lawrence-forooghian lawrence-forooghian deleted the chat-attaching-ops branch November 25, 2024 11:56
@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 28, 2024

@AndyTWF Btw, do we not check for ATTACHED state for presence leave as per CHA-PR4 ?

@AndyTWF
Copy link
Contributor Author

AndyTWF commented Nov 28, 2024

@AndyTWF Btw, do we not check for ATTACHED state for presence leave as per CHA-PR4 ?

Yes - will add.

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 28, 2024 •
edited
Loading

The same goes for CHA-T4a1 ( typing presence enter ) and CHA-T4a2 ( explicit typing.stop() - typing presence leave )

@sacOO7
Copy link
Collaborator

sacOO7 commented Nov 28, 2024

Simply, any room operation triggered by the user, that sends msg over realtime connection should check for RoomStatus.ATTACHED

@sacOO7 sacOO7 mentioned this pull request Nov 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sacOO7 sacOO7 sacOO7 approved these changes

@owenpearson owenpearson Awaiting requested review from owenpearson owenpearson was automatically assigned from ably/chat-reviewers

@JoaoDiasAbly JoaoDiasAbly Awaiting requested review from JoaoDiasAbly JoaoDiasAbly was automatically assigned from ably/chat-reviewers

@lawrence-forooghian lawrence-forooghian Awaiting requested review from lawrence-forooghian

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Presence spec points assume ATTACHING status is always caused by an ATTACH operation
3 participants
@AndyTWF @sacOO7 @lawrence-forooghian