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.

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

Don't join on kicking yourself #250

Merged
merged 4 commits into from
Oct 13, 2020
Merged

Don't join on kicking yourself #250

merged 4 commits into from
Oct 13, 2020

Conversation

Half-Shot
Copy link
Contributor

I've also taken the liberty to add a userId getter to the class, as we use it all over the place. I've also added support for folks to leave with a reason. Eventually we could use the leave call, but the JS SDK doesn't let us use a reason. You can leave with a reason in the matrix spec, so it's fine to send a kick instead for now.

@Half-Shot Half-Shot force-pushed the hs/dont-join-on-kick branch from 209d759 to 6efe4ad Compare October 12, 2020 19:04
@Half-Shot Half-Shot requested review from a team and benparsons and removed request for a team October 12, 2020 19:18
Copy link
Member

@jaywink jaywink left a comment

Choose a reason for hiding this comment

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

Code change looks solid.

Some reservations on kicking oneself just to show a reason, seems ... wrong :) But if the client doesn't yet support leaving with a reason then I guess it makes sense for now at least.

@Half-Shot
Copy link
Contributor Author

Some reservations on kicking oneself just to show a reason, seems ... wrong :) But if the client doesn't yet support leaving with a reason then I guess it makes sense for now at least.

The resulting state is identical, so matrix client's wont know any difference. It's just a bit yuck from a "we're calling kick to do a leave" perspective.

@Half-Shot Half-Shot merged commit cb860cf into develop Oct 13, 2020
@Half-Shot Half-Shot deleted the hs/dont-join-on-kick branch May 2, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants