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

Enhancements for the Authentication mechanisms documentation #30658

Conversation

MichalMaler
Copy link
Contributor

@MichalMaler MichalMaler commented Jan 27, 2023

QDOCS-93: Authentication mechanisms revamping

  • Fixing the names to match diataxes style
  • Adding a missing ID
  • Cleaning content and moving it to dedicated sections
  • Adding tables for the complete overview
  • Style/grammar fixes

Initial preview for tables movements:
https://github.com/quarkusio/quarkus/blob/a3d55ddf0f11b353d0835e76760de5fe9e2ef75a/docs/src/main/asciidoc/security-authentication-mechanisms-concept.adoc

Renaming/Redirecting PR for these changes:
quarkusio/quarkusio.github.io#1623

Signed-off-by: Michal Maléř mmaler@redhat.com

@MichalMaler
Copy link
Contributor Author

@sberyozkin @jmartisk Hello Sergey and Jan! Here are some changes for the mentioned section. It is possible that we will need to move the revamped tables to a more specific sections, and I will provide a final review when this shifts will be done.

Xrefs are currently not tested yet. Will do it as a final step

Can I ask you for a review?

Thank You.

MichalMaler added a commit to MichalMaler/quarkusio.github.io that referenced this pull request Jan 27, 2023
Adding a new file-name for the file renamed in:
quarkusio/quarkus#30658
@github-actions
Copy link

github-actions bot commented Jan 27, 2023

🙈 The PR is closed and the preview is expired.

@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch from 941e1d4 to b40e040 Compare January 27, 2023 13:48
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

@MichalMaler Thanks for going ahead with creating a new table, IMHO it would be useful, a left a few comments.
By the way, please tweak the mutual TLS link at the very top of the builtin mechanisms section. I'm not sure though we need those Basic/Form/MTLS links given that right below them Basic/Form/MTLS subsections start ? Michelle, @michelle-purcell would you like to retain them ?

@MichalMaler
Copy link
Contributor Author

@MichalMaler Thanks for going ahead with creating a new table, IMHO it would be useful, a left a few comments. By the way, please tweak the mutual TLS link at the very top of the builtin mechanisms section. I'm not sure though we need those Basic/Form/MTLS links given that right below them Basic/Form/MTLS subsections start ? Michelle, @michelle-purcell would you like to retain them ?

@sberyozkin I fixed the link. Will remove them if @michelle-purcell agrees they are redundant or just residuals after the content movement :) Let me know.

@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch 2 times, most recently from 6e8e37a to 57c2249 Compare January 30, 2023 10:08
@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch 5 times, most recently from 31400ad to 40ada83 Compare January 30, 2023 11:52
@MichalMaler
Copy link
Contributor Author

@sberyozkin I think we are ready to merge.
The xref:security-oidc-code-flow-authentication-concept.adoc[OIDC Web] link will be working when the Sheila's PR (#30518) is merged.

@sheilamjones
Copy link
Contributor

Hi @MichalMaler, good job on the restructuring here. I just had a couple of minor comments and suggestions.
Kind regards,
Sheila

@MichalMaler
Copy link
Contributor Author

Hi @MichalMaler, good job on the restructuring here. I just had a couple of minor comments and suggestions. Kind regards, Sheila

Thank you for quick and great review, as always :) Applied most of your suggestions. GJ! @sheilamjones

@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch from d0f4196 to b29f5df Compare January 31, 2023 17:28
Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

IMHO it is now nearly ready to go.

@sheilamjones Please update your PR when you get a chance as Michal's PR points to the file which will only be resolvable once your PR gets merged :-), so for now please don't merge

Quarkus Documentation automation moved this from To do to Reviewer approved Jan 31, 2023
@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch 5 times, most recently from a3d55dd to 905ff8d Compare February 1, 2023 13:44
@MichalMaler
Copy link
Contributor Author

Pushed the latest update:
image

@sberyozkin
Copy link
Member

sberyozkin commented Feb 2, 2023

Hey @MichalMaler Sheila's PR is now on the main branch, can you please rebase and check the links, FYI, the OIDC web app concept doc has been renamed, so some minor sync will be needed for your PR, thanks

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Apply suggestions from code review

Co-authored-by: sberyozkin <sberyozkin@gmail.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Apply suggestions from code review

Co-authored-by: jherrman <jherrman@redhat.com>

Fixes from a peer review

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>
@MichalMaler MichalMaler force-pushed the QDOCS-93-Authentication-mechanisms-revamping branch from 905ff8d to af69936 Compare February 3, 2023 08:59
@MichalMaler
Copy link
Contributor Author

@sberyozkin Hello Sergey! Thx for letting me know. One additional xref fixed. The whole PR now using the new names that reflects Sheila's work.
We did some good job here again. Cheers!

@MichalMaler
Copy link
Contributor Author

This should be merged too, but it is not my call:
quarkusio/quarkusio.github.io#1623

@sberyozkin
Copy link
Member

@MichalMaler Great work indeed, thanks very much

@sberyozkin sberyozkin merged commit 79d60e1 into quarkusio:main Feb 3, 2023
Quarkus Documentation automation moved this from Reviewer approved to Done Feb 3, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Feb 3, 2023
@MichalMaler MichalMaler deleted the QDOCS-93-Authentication-mechanisms-revamping branch February 3, 2023 14:11
@gsmet
Copy link
Member

gsmet commented Feb 28, 2023

I'm unmarking all the doc PRs for backports. They are all part of massive changes that shouldn't be backported. Please don't mark for backport massive structural changes to the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

7 participants