-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
Factor out all the common parameters of the various /relations apis #1745
Conversation
Apparently: "the ref partial doesn't support references using fragments yet" |
This writes up matrix-org/matrix-spec-proposals#3981 Hopefully this is relatively straightforward, apart from having to add the parameters and response field in all three places. I tried to factor these out but it seems references just aren't supported in the right places currently (see #1745 for my efforts). Path parameters can't be optional, so it can't be done that way either.
Hopefully I've fixed this in #1749, which I have now merged in here. |
5fe8d21
to
f16d804
Compare
Awesome, thank you for doing that. So this just needs a newsfile and it's ready to go? |
Ah no, I'll add a checklist to the description. |
sorry, I'm still thrashing this out with @zecakeh |
For info, I did a bit more refactoring in zecakeh@bd54781 to check more uses of |
Right, #1751 has landed, so I'm going to rebase this again |
This doesn't currently work, perhaps because the template needs to resolve refs? Also, the other instances of the params need to be replaced with the refs and the response fields probably should be factored out too.
089163a
to
5a442c4
Compare
Contributed by @zecakeh zecakeh@bd54781.
* Spec for MSC3981 This writes up matrix-org/matrix-spec-proposals#3981 Hopefully this is relatively straightforward, apart from having to add the parameters and response field in all three places. I tried to factor these out but it seems references just aren't supported in the right places currently (see #1745 for my efforts). Path parameters can't be optional, so it can't be done that way either. * Missed schemas * newsfile * Actually it clearly isn't going to support markdown, is it? * grammar Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * grammar Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Clarity Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Clarity Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Typo Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * More clarity. Note this is counter what the MSC actually proposed to add, but I think it's clear that this is what it meant. --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
This doesn't currently work, perhaps because the template needs to resolve refs?
Things this PR needs:
Preview: https://pr1745--matrix-spec-previews.netlify.app