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

replace "<content>" with "content" #1370

Merged
merged 4 commits into from
Jan 16, 2023
Merged

Conversation

dylhack
Copy link
Contributor

@dylhack dylhack commented Dec 1, 2022

This parameter that's part of the content-repo OpenAPI spec causes generators to mess up.

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

This parameter that's part of the content-repo openapi spec causes generators to mess up
@dylhack dylhack requested a review from a team as a code owner December 1, 2022 21:09
@dylhack
Copy link
Contributor Author

dylhack commented Dec 1, 2022

note: #1310 fixes this

@turt2live
Copy link
Member

if #1310 fixes this, would it be better to pursue that option instead?

@dylhack
Copy link
Contributor Author

dylhack commented Dec 1, 2022

Yes if it were quicker to merge. In this case #1310 can manage the conflicts as long as that means code generators work today.

@turt2live
Copy link
Member

For the speed the spec moves at, #1310 is likely to land before the next spec release at the current rate - this seems suitable for conde generators too, as using unstable API definitions is, well, unstable.

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes looks good but please add a news fragment, as described here: https://github.com/matrix-org/matrix-spec/blob/main/CONTRIBUTING.rst#adding-to-the-changelog

@dylhack dylhack requested a review from KitsuneRal January 13, 2023 22:14
@KitsuneRal
Copy link
Member

This snippet doesn't even land in the spec text, it seems - and #1310 will have to be updated (merged or rebased) to the latest main anyway, so I suggest merging it. Up to @turt2live though.

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.

otherwise lgtm, as it sounds like @KitsuneRal is okay with managing conflicts

changelogs/internal/newsfragments/1370.clarification Outdated Show resolved Hide resolved
Co-authored-by: Travis Ralston <travpc@gmail.com>
@dylhack
Copy link
Contributor Author

dylhack commented Jan 16, 2023

Awesome thanks

@turt2live
Copy link
Member

For future changes, please note that we normally require Sign Off to ensure the change can be incorporated into the project. For extraordinarily trivial changes like this however, we tend to bypass this requirement.

@turt2live turt2live enabled auto-merge (squash) January 16, 2023 19:50
@turt2live turt2live merged commit f06ffc8 into matrix-org:main Jan 16, 2023
clokep pushed a commit to clokep/matrix-spec that referenced this pull request May 3, 2023
* replace "<content>" with "content"

This parameter that's part of the content-repo openapi spec causes generators to mess up

* added changelogs

* Update changelogs/internal/newsfragments/1370.clarification

Co-authored-by: Travis Ralston <travpc@gmail.com>

Co-authored-by: Travis Ralston <travpc@gmail.com>
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.

3 participants