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

Reorganize event structure in c2s spec and clarify event capabilities #2087

Merged
merged 4 commits into from Jun 11, 2019

Conversation

Projects
None yet
2 participants
@turt2live
Copy link
Member

commented Jun 6, 2019

Fixes #1166
Fixes #1527
Fixes #1827

Note: In order to fix the "state events have the following fields: [no words]" bug (1827) we need to resolve references on common event types. When doing this we ultimately end up with more fields than may be required to explain the section, however this commit alters the section descriptions to just say "these fields" instead of "these additional fields".

This is also preferable over trying to get the inheritance reversed in the common event types, as the /sync endpoint has a high amount of reliance on partial events definitions.

Reorganize event structure in c2s spec and clarify event capabilities
Fixes #1166
Fixes #1527
Fixes #1827

Note: In order to fix the "state events have the following fields: [no words]" bug (1827) we need to resolve references on common event types. When doing this we ultimately end up with more fields than may be required to explain the section, however this commit alters the section descriptions to just say "these fields" instead of "these additional fields".

This is also preferable over trying to get the inheritance reversed in the common event types, as the `/sync` endpoint has a high amount of reliance on partial events definitions.

@turt2live turt2live added the Matrix 1.0 label Jun 6, 2019

@turt2live turt2live requested a review from matrix-org/spec-core-team Jun 6, 2019

@ara4n

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

lgtm, depending on what it actually looks like with the expanded "events have these fields", "state events have these fields" style tables. hopefully the duplication isn't too painful.

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

@ara4n

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

yeah, it's not ideal - ~10 out of the ~12 fields are the same?

could we just cheat and hardcode it rather than generating it from the swagger, to avoid it being icky?

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

Yea, I'll just hardcode the state events section. The room events can probably stay autogenerated because they have the most fields.

@turt2live turt2live removed the request for review from matrix-org/spec-core-team Jun 10, 2019

@turt2live turt2live requested a review from ara4n Jun 10, 2019

@ara4n

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

needs moar prev_content

@turt2live

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

this was lgtm'd out of band

@turt2live turt2live merged commit 2d18f81 into master Jun 11, 2019

8 checks passed

buildkite/matrix-doc Build #323 passed (52 seconds)
Details
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

@turt2live turt2live deleted the travis/1.0/events-are-extensible branch Jun 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.