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

clarify federation Authorization header an add destination property #1067

Merged
merged 6 commits into from
May 30, 2022

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented May 17, 2022

@turt2live
Copy link
Member

@uhoreg is this ready for review?

@uhoreg uhoreg marked this pull request as ready for review May 27, 2022 22:05
@uhoreg uhoreg requested a review from a team as a code owner May 27, 2022 22:05
@uhoreg
Copy link
Member Author

uhoreg commented May 27, 2022

Should be ready now

@@ -2,7 +2,7 @@
{{ $this := .Params.this }}

{{ if $this }}
<span>**[New in this version]**</span>
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes are needed, otherwise it displays the markdown in the federation docs, rather than the HTML

Copy link
Member

Choose a reason for hiding this comment

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

hmm.. that's surprising. What version of hugo?

Copy link
Member Author

Choose a reason for hiding this comment

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

It happened with whatever version of hugo CI was using. It worked fine in the CS bits. I assume it's because it got substituted in, and then the file got included in the main file, and then reprocessed. But it didn't work for the federation doc.

Copy link
Member

Choose a reason for hiding this comment

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

right, yea. {{< shortcode >}} is a literal while {{% shortcode %}} is interpreted - we need to use angle brackets to get the inline stuff to work, annoyingly.

Copy link
Member

Choose a reason for hiding this comment

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

This is fun though (and not worth fixing in this PR, I think):

s2s spec:
image

c2s spec:
image

The code blocks are different. I think it's because of the table structure, but not really sure.

@jcgruenhage
Copy link
Contributor

#511 which is sort-of related will be solved separately, because it's a bigger change I'll assume? Ofher than that, this looks hood to me. Thanks!

@uhoreg
Copy link
Member Author

uhoreg commented May 28, 2022

#511 which is sort-of related will be solved separately, because it's a bigger change I'll assume?

Oh, I didn't notice that one. Yes, I think that would be a separate PR. I think I'm already stretching it, making three changes in 1 PR.

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 - thanks!

content/server-server-api.md Outdated Show resolved Hide resolved
content/server-server-api.md Outdated Show resolved Hide resolved
content/server-server-api.md Outdated Show resolved Hide resolved
@@ -2,7 +2,7 @@
{{ $this := .Params.this }}

{{ if $this }}
<span>**[New in this version]**</span>
Copy link
Member

Choose a reason for hiding this comment

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

This is fun though (and not worth fixing in this PR, I think):

s2s spec:
image

c2s spec:
image

The code blocks are different. I think it's because of the table structure, but not really sure.

@turt2live turt2live added the release-blocker Blocks the next release from happening label May 28, 2022
@turt2live turt2live merged commit 2780055 into matrix-org:main May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Blocks the next release from happening
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Exact Federation Authorization header format isn't clear
3 participants