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

Renamed doc filename and xrefs to include token #33502

Merged
merged 1 commit into from
May 23, 2023

Conversation

michelle-purcell
Copy link
Contributor

@michelle-purcell michelle-purcell commented May 19, 2023

Renamed the doc source file security-oidc-bearer-authentication-concept.adocsecurity-oidc-bearer-token-authentication-concept.adoc and all xrefs to it.

Added a URL redirect also in the quarkusio.github.io repo for when this change goes live on the doc portal site: 1708

@michelle-purcell
Copy link
Contributor Author

@sberyozkin - Fixing this deferred SME request from the last phase.
(@sheilamjones - FYI)

@sberyozkin
Copy link
Member

Hi @michelle-purcell, thanks very much, it look fine, just a quick question. Is it important to have -concept in the doc name ? For example, security-oidc-bearer-token-authentication.adoc seems quite precise, security-oidc-bearer-token-authentication-concept.adoc is OK, but do we have to express this doc is concept in the actual doc name ?
If it is the best practice then yeah, lets do it, if we can optimize then I can create another issue afterwards so that we can drop -concept in all the concept doc file names ?

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.

Thanks Michelle for taking care of it

@michelle-purcell
Copy link
Contributor Author

@sberyozkin - Thanks Sergey for reviewing.

Today, the Quarkus doc contributor guidelines and templates request the content type at the end. I think the quarkusio doc landing page builder (index) uses either the filename or ID to determine the content type heading to place the guide under.

But... in saying that. I like this suggestion to remove the content type from the end of a file name. I think it creates extra churn and longer file names. Also, the diataxis framework recognizes that content types are not one-size fits all, and the guidelines do acknowledge cross-pollination in the 'real world'. Other organizations that do diataxis don't include the content type in the filename.

@ebullient - WDYT? Should we remove the filename guideline to always include content type or would that pose a big issue with the implementation?

@sberyozkin
Copy link
Member

Yes, the actual doc content makes it clear what it is, Concept, How To etc. At the URL level, the shorter URL the easier it is to follow them, so if we can optimize a bit and remove Diataxis qualifiers from the file names it would be not bad IMHO.

May be it is better to open a dedicate issue to discuss.

Cheers

@github-actions
Copy link

github-actions bot commented May 19, 2023

🙈 The PR is closed and the preview is expired.

@ebullient
Copy link
Member

It's fine. I was using the filename as a way to tell if the content had been revised or not.. (that is where the classification for the site, the type, originally comes from).

We could use a category instead.. (and verify that the category doesn't appear twice). In the case of similarly named files, the suffix can be useful to disambiguate, but I agree that's probably case by case.

@sberyozkin
Copy link
Member

Thanks @michelle-purcell @ebullient, I can open an issue to review which file names for the already refactored docs might be optimized

@sberyozkin
Copy link
Member

@michelle-purcell I'll keep this PR open till Monday - I'm just not sure right now how you'd prefer to handle a possible optimization. If you and Erin agree then another option might be to update this PR and #1708. So I'll wait till Monday...

Thanks

@michelle-purcell michelle-purcell force-pushed the enhance-security-docs branch from bedcd92 to 6b1f3be Compare May 23, 2023 07:52
@michelle-purcell
Copy link
Contributor Author

michelle-purcell commented May 23, 2023

@sberyozkin, @RuanNunes, @ebullient - Thanks.
Sorry for slow reply. I was on PTO yesterday. 👍 +1 from me to merging this one as-is and then removing the diataxis content type from all doc files that need it as a separate issue and PR. Would be good to test this first also.

@sberyozkin
Copy link
Member

Hey @michelle-purcell Np at all, sounds good, let me merge it now, and then I'll open another issue to consider a possible optimization, thanks

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

Successfully merging this pull request may close these issues.

4 participants