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

remove references to presence lists #1817

Merged
merged 4 commits into from
Feb 1, 2019
Merged

remove references to presence lists #1817

merged 4 commits into from
Feb 1, 2019

Conversation

neilisfragile
Copy link
Contributor

Relates #1810

@neilisfragile neilisfragile requested a review from a team January 28, 2019 15:12
@turt2live
Copy link
Member

Something like this surely needs an MSC as it's a spec change? It'd be even better if the feature were replaced by something more usable as well.

@neilisfragile
Copy link
Contributor Author

neilisfragile commented Jan 28, 2019 •
edited by turt2live
Loading

MSC

turt2live
turt2live requested changes Jan 28, 2019
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

This overall LGTM for once the MSC lands. In the meantime, we should also remove the schemas for those EDUs if we don't plan on using them.

KitsuneRal
KitsuneRal approved these changes Jan 29, 2019
View reviewed changes
Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

LGTM, aside from one typo and assuming there are no other places referring to presence list.

specification/modules/presence.rst Outdated
A presence list is a list of user IDs whose presence the user wants to follow.
To be added to this list, the user being added must be invited by the list owner
who must accept the invitation.
and transmitted as an ``m.presence`` event. Presence event are sent to
Copy link
Member

Choose a reason for hiding this comment

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

Either 'Presence event is' or 'Presence events are'.

turt2live
turt2live approved these changes Jan 31, 2019
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Thanks for updating this. LGTM once the MSC clears FCP (assuming no changes)

@turt2live
Copy link
Member

(landing this with anticipation that the FCPs are going to clear just fine - if this changes, we'll do the needed)

@turt2live turt2live merged commit e27c83d into master Feb 1, 2019
@turt2live turt2live mentioned this pull request Feb 4, 2019
Closed
jevolk added a commit to jevolk/charybdis that referenced this pull request Aug 16, 2019
@jevolk
modules/client/presence: Remove presence lists stub. (matrix-org/matr…
183241b 
…ix-spec-proposals#1817)
@Half-Shot Half-Shot mentioned this pull request Jun 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KitsuneRal KitsuneRal KitsuneRal approved these changes

@turt2live turt2live turt2live approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neilisfragile @turt2live @KitsuneRal