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

room_type is not a required parameter in practice #1110

Merged

Conversation

nico-famedly
Copy link
Contributor

@nico-famedly nico-famedly commented Jun 7, 2022

In practice servers seem to mirror what the room create event does and
leave out the room_type when unset.

I don't think the intention of the spec was to have the room type required
on normal rooms and the text seems to match that too.

Signed-off-by: Nicolas Werner n.werner@famedly.com

Preview: https://pr1110--matrix-spec-previews.netlify.app

In practice servers seem to mirror what the room create event does and
leave out the room_type when unset.

Signed-off-by: Nicolas Werner <n.werner@famedly.com>
@nico-famedly nico-famedly requested a review from a team as a code owner June 7, 2022 13:28
Signed-off-by: Nicolas Werner <n.werner@famedly.com>
@clokep
Copy link
Contributor

clokep commented Jun 7, 2022

This seems to match the language in MSC2946:

room_type: the value of the type field from the room's m.room.create event, if any.

Not sure if the server-server API doc needs to be updated too or if it re-uses this same chunk?

@nico-famedly
Copy link
Contributor Author

Yeah, it also matches the spec text (which has that exact text). Didn't check the server-server API, let me do that.

@nico-famedly
Copy link
Contributor Author

Yes, the server-server APi has that field as required too. It also lists allowed_roomids and such as required, which contradicts the spec text as well, let me update that.

They are optional according to the text, but the openapi marks them as
required instead.

Signed-off-by: Nicolas Werner <n.werner@famedly.com>
@nico-famedly
Copy link
Contributor Author

Done. room_type and allowed_roomids are optional according to the text a few lines above the field marking them as required.

Signed-off-by: Nicolas Werner <n.werner@famedly.com>
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!

@turt2live turt2live merged commit e70045f into matrix-org:main Jun 7, 2022
td-famedly pushed a commit to famedly/dart_matrix_api_lite that referenced this pull request Mar 22, 2024
BREAKING CHANGE (because from is now optional, so it can't be specified
conditionally)

See also:

- matrix-org/matrix-spec#1110
- matrix-org/matrix-spec#1097
td-famedly pushed a commit to famedly/dart_matrix_api_lite that referenced this pull request Mar 22, 2024
BREAKING CHANGE (because from is now optional, so it can't be specified
conditionally)

See also:

- matrix-org/matrix-spec#1110
- matrix-org/matrix-spec#1097
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.

None yet

3 participants