-
Notifications
You must be signed in to change notification settings - Fork 383
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
Remove 'room_id' field from m.typing
, m.receipt
and m.fully_read
examples and schema
#3679
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change feels wrong - a room_edu
is in a room, so needs a room ID. Not all EDUs use a top-level room_id
though, so possibly just a badly written example somewhere specific?
I think the confusion here stems from the fact that typing events (and, for that matter, EDUs in general) have a different format when sent over the federation API (where they do have a I haven't checked, but I think this file is only used when rendering the |
@richvdh I believe so, though CI seems to be unhappy. We should perhaps rename this in some way so that it's explicitly client-specific? |
The CI is failing because the schema and the example don't match - this PR only touches the example, not the schema. |
m.typing
, m.receipt
and m.fully_read
examples and schema
I have updated the schema, and CI now passes. I also discovered that I've only been verifying against Synapse/Element here. I need to check other implementations to see if these fields are being relied upon. |
This should fix #3183 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - just need a changelog to denote the bugfix in the spec prose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
…` examples and schema (matrix-org#3679) The spec had an erroneous `room_id` field in a m.typing EDU entry of /sync, `m.read` receipts in `/sync`, and `m.fully_read` room account data objects in the spec. None of these are necessary nor used in practice. Checking part of the ecosystem for whether clients look for, or homeservers include, these room_id fields, I found that: Element does not require them, nor does Synapse include them. Ruma does not include them. Dendrite does not include them. nheko/mtxclient does not look for them. This change removes room_id from the example and OpenAPI schema in each case mentioned above. It only affects the Client-Server spec - the Server-Server spec text remains unchanged. The field was initially introduced in 0f28f83.
It was discovered here that the spec had an erroneous
room_id
field in am.typing
EDU entry of/sync
. After some further research,room_id
fields also appear inm.read
receipts in/sync
, andm.fully_read
room account data objects in the spec. None of these are necessary nor used in practice.Checking part of the ecosystem for whether clients look for, or homeservers include, these
room_id
fields, I found that:This change removes
room_id
from the example and OpenAPI schema in each case mentioned above. It only affects the Client-Server spec - the Server-Server spec text remains unchanged.The field was initially introduced in 0f28f83#diff-8d917764f05b0823bc6aee1f9a74895d38df32fb1eeb10c18a55d9120cf23cb2R3.
Preview: https://pr3679--matrix-org-previews.netlify.app