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

Add missing enum fields for the set_presence parameter #780

Merged
merged 4 commits into from
Aug 30, 2018
Merged

Add missing enum fields for the set_presence parameter #780

merged 4 commits into from
Aug 30, 2018

Conversation

mujx
Copy link
Contributor

@mujx mujx commented Jan 5, 2017

No description provided.

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@richvdh
Copy link
Member

richvdh commented Mar 8, 2017

ok to test

@richvdh
Copy link
Member

richvdh commented Mar 8, 2017

@mujx: are you sure? What makes you believe that other values are valid here?

The description only documents offline, so if other values are valid, they need documenting too.

@mujx
Copy link
Contributor Author

mujx commented Mar 8, 2017

I thought that the presence parameter would support all the values from the presence endpoints.. If I'm wrong and only one value is allowed why not convert it into a boolean?

@leonerd
Copy link
Contributor

leonerd commented Jun 5, 2017

I believe all the values should be allowed. At least, that was my intention when I designed it :)

@maxidorius
Copy link
Contributor

@richvdh Any update on this?

@richvdh
Copy link
Member

richvdh commented Oct 26, 2017

The description only documents offline, so if other values are valid, they need documenting too.

@turt2live
Copy link
Member

@mujx are you able to update this to include descriptions? Given the age, it'd be nice to have this be merged :)

@turt2live turt2live self-assigned this Aug 14, 2018
@turt2live
Copy link
Member

@mujx Thank you for this PR! I hope you don't mind that I've added the requested descriptions to the PR. Stuff like this is very much part of the "end of August" goal for the spec, which happens to be in just a few short days.

It's worth noting that synapse currently does not support unavailable as an option, however as per leo's comment above, it seems reasonable to expect it in the spec and have the implementation follow.

I've also merged the latest master into the branch so that the PR can benefit from the new changelog format and build process.

@turt2live turt2live requested a review from a team August 28, 2018 20:31
@turt2live turt2live removed their assignment Aug 28, 2018
@mujx
Copy link
Contributor Author

mujx commented Aug 29, 2018

Thanks @turt2live for finishing the PR. I've forgotten about the PR because I wasn't sure what unavailable really means in that context.

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.

7 participants