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

Conditionalize paragraph that links to conditional content #41914

Closed
wants to merge 1 commit into from

Conversation

rolfedh
Copy link
Contributor

@rolfedh rolfedh commented Jul 16, 2024

Enhance the documentation's clarity and conditional logic by enclosing cross-references that link to the ifndef::no-quarkus-oidc-token-propagation-reactive[] section within the same conditionals. Otherwise, the cross references are broken when the condition is asserted.

@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Jul 16, 2024
@quarkus-bot

This comment has been minimized.

@sberyozkin
Copy link
Member

@rolfedh I've approved, but then I thought, should we have this conditional content on main two for these extensions ? I see way it may have to be the case for 3.8 but what about main ?

@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 16, 2024

@rolfedh I've approved, but then I thought, should we have this conditional content on main two for these extensions ? I see way it may have to be the case for 3.8 but what about main ?

Good question, @sberyozkin. Currently, main includes conditional content for these two extensions. To maintain consistency and ensure main serves as the current source of truth, I propose we add this change now. If we confirm product support for any extension in the future, I'll remove the corresponding conditionalization tags from all affected content. WDYT?

@sberyozkin
Copy link
Member

Sure, but is it the current state of truth though for main, which is an upstream thing, with both of these extensions to be supported at the next RHBQ level...
Let me CC @rsvoboda

@rsvoboda
Copy link
Member

@rolfedh we need to understand if this work is related to 3.8 or to the future RHBQ feature release.

@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 16, 2024

@rolfedh we need to understand if this work is related to 3.8 or to the future RHBQ feature release.

@rsvoboda,

This PR relates to main only. I have created #41921 for 3.8.

Could you please confirm if we have tested or can guarantee support for the following extensions in RHBQ 3.14?

  • quarkus-oidc-token-propagation (also known as quarkus-resteasy-client-oidc-token-propagation)
  • quarkus-oidc-token-propagation-reactive (also known as quarkus-rest-client-oidc-token-propagation)

If support is confirmed, I suggest we close this PR and I will create a new one to remove this type of conditionalization from the other content. Otherwise, I recommend merging this PR now and removing the conditionalization once support is verified. WDYT?

@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 16, 2024

@rsvoboda, maintaining two separate realities in main can lead to inconsistencies where the extension is supported in some content but not in others. It would be more effective to have a consistent approach throughout.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 16, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 128f600.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@rolfedh rolfedh marked this pull request as draft July 16, 2024 17:53
@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 16, 2024

@rsvoboda @sberyozkin, for main and the upcoming 3.14 release, what are your thoughts on this PR instead: PR #41938?

@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 16, 2024

@sberyozkin @rsvoboda let's close this one and use #41938 instead, okay?

@rsvoboda
Copy link
Member

@rolfedh
quarkus-oidc-token-propagation and quarkus-oidc-token-propagation-reactive are tracked https://issues.redhat.com/browse/QUARKUS-4179, the target was 3.8.5, tests plan was done, tests were added, functional testing passed, the only trouble that remains is that extension atrifacts are not productized even though the tech preview support metadata are present.
https://code.quarkus.redhat.com/?extension-search=origin:platform%20token-propagation

So I would treat them as supported extenstions (with tech preview level) so the parts about quarkus-oidc-token-propagation(-reactive) should be available in 3.8 and 3.15 documentation.

@rolfedh
Copy link
Contributor Author

rolfedh commented Jul 17, 2024

Not needed. The quarkus-oidc-token-propagation and quarkus-oidc-token-propagation-reactive extensions have tech preview (supported) status in 3.8 and later.

@rolfedh rolfedh closed this Jul 17, 2024
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jul 17, 2024
Copy link

🙈 The PR is closed and the preview is expired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docstyle issues related for manual docstyle review area/documentation triage/invalid This doesn't seem right
Projects
Development

Successfully merging this pull request may close these issues.

3 participants