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.

Sign up for GitHub

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

Jump to bottom

clarify federation Authorization header an add destination property #1067

Merged
merged 6 commits into from May 30, 2022
Merged

clarify federation Authorization header an add destination property #1067

merged 6 commits into from May 30, 2022

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented May 17, 2022 •
edited by github-actions bot

@uhoreg uhoreg mentioned this pull request May 17, 2022
Closed
@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

uhoreg
uhoreg commented May 27, 2022
View reviewed changes
layouts/shortcodes/added-in.html
@@ -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.

@uhoreg uhoreg mentioned this pull request May 27, 2022
Merged
@turt2live turt2live added this to Requires spec review (post-FCP) in Spec Core Team Backlog May 27, 2022
@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.

turt2live
turt2live approved these changes May 28, 2022
View reviewed changes
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
layouts/shortcodes/added-in.html
@@ -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 mentioned this pull request May 30, 2022
Merged
@turt2live turt2live merged commit 2780055 into matrix-org:main May 30, 2022
@turt2live turt2live moved this from Requires spec review (post-FCP) to Done to some definition in Spec Core Team Backlog May 30, 2022
@zecakeh zecakeh mentioned this pull request Jun 3, 2022
Closed
66 tasks
@S7evinK S7evinK mentioned this pull request Sep 29, 2023
Open
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turt2live turt2live turt2live approved these changes

Assignees
No one assigned
Labels
release-blocker Blocks the next release from happening
Projects
Spec Core Team Backlog
  
Done to some definition
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Exact Federation Authorization header format isn't clear
3 participants
@uhoreg @turt2live @jcgruenhage