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

Conversation

7 participants
@mujx
Contributor

mujx commented Jan 5, 2017

No description provided.

@matrixbot

This comment has been minimized.

Member

matrixbot commented Jan 5, 2017

Can one of the admins verify this patch?

Add missing enum fields for the set_presence parameter
Signed-off-by: Konstantinos Sideris <siderisk@auth.gr>
@richvdh

This comment has been minimized.

Member

richvdh commented Mar 8, 2017

ok to test

@richvdh

This comment has been minimized.

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

leonerd commented Jun 5, 2017

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

@maxidor

This comment has been minimized.

Contributor

maxidor commented Oct 26, 2017

@richvdh Any update on this?

@richvdh

This comment has been minimized.

Member

richvdh commented Oct 26, 2017

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

@turt2live

This comment has been minimized.

Member

turt2live commented Jul 4, 2018

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

@turt2live turt2live added this to Needs review in August 2018 r0 Aug 14, 2018

@turt2live turt2live moved this from Needs review to In progress in August 2018 r0 Aug 14, 2018

@turt2live turt2live moved this from In progress to In review in August 2018 r0 Aug 14, 2018

@turt2live turt2live self-assigned this Aug 14, 2018

@turt2live

This comment has been minimized.

Member

turt2live commented Aug 28, 2018

@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 matrix-org/spec-core-team Aug 28, 2018

@turt2live turt2live removed their assignment Aug 28, 2018

@mujx

This comment has been minimized.

Contributor

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.

August 2018 r0 automation moved this from In review (just the PRs) to Reviewer approved Aug 30, 2018

@uhoreg

uhoreg approved these changes Aug 30, 2018

@turt2live turt2live merged commit f04afaa into matrix-org:master Aug 30, 2018

7 checks passed

ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details

August 2018 r0 automation moved this from Reviewer approved to Done (this list will be incomplete) Aug 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment