This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
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.
Add a new module API to update user presence state. #16544
Add a new module API to update user presence state. #16544
Changes from all commits
99aa721
be16e5b
a44f1c6
b1c55ee
eef5b41
747e570
d512c87
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
track_presence
is very clear---thank you!I think the phrase "Whether presence is enabled" is still a bit confusing. Can we call
self.presence_enabled
something likeself.report_presence
? (I'm guessing that this bool now controls the reporting of presence down /sync and in response to explicit presence GETs).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It ends up getting used a bit internally too, I'm not sure that
presence_enabled
is the best name, but I thinkreport_presence
doesn't quite capture it either.So it gets used for:
/presence
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
report_presence
easily covers 1 & 2, it doesn't feel quite right for 3 & 4 though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blimey, that's a lot to juggle. Let's leave it as-is then. If nothing else, we'll have these comments as signposts for our future selves!