-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
DOCS: Add downstream-relevant attributes to facilitate single sourcing #45962
Conversation
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
Hi @gsmet, @rolfedh, @iocanel, |
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with gsmet's suggestions. Otherwise, everything looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the changes requested by @gsmet
0c06f41
to
f411b57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @iocanel. I have now implemented these fixes.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f411b57
to
609a6e6
Compare
This comment has been minimized.
This comment has been minimized.
@gsmet: it seems that your change requests have been applied, can we merge this? |
:name-image-ubi8-open-jdk-17: registry.access.redhat.com/ubi8/openjdk-17 | ||
:name-image-ubi8-open-jdk-17-short: ubi8/openjdk-17 | ||
:name-image-ubi8-open-jdk-21: registry.access.redhat.com/ubi8/openjdk-21 | ||
:name-image-ubi8-open-jdk-21-short: ubi8/openjdk-21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this patch was written, we moved to Ubi 9 so this needs adjusment (included in future 3.20).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gsmet, I updated these values to Ubi 9 per your suggestion. Can I clarify though that this will be correct for RHBQ 3.20 (which is based on upstream 3.19)? This PR is aimed to be backported to 3.19.
Regards,
Sheila
609a6e6
to
be67484
Compare
This comment has been minimized.
This comment has been minimized.
Signed-off-by: shjones <[email protected]>
be67484
to
8e10ae3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirm UBI 9 is the version in 3.20.
:jdk-version-other: 17 | ||
:jdk-version-latest: 21 | ||
:jdk-version-all: 17 or 21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this to make progress but... I don't think this will fly but it's just an intuition as I have no idea how you plan to use them.
In the community, people can use any Java 17+ (typically, if they want to use Java 23, that's fine).
So please send me an email when you start using these, with a pointer, so that I can have a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gsmet,
Thanks for your review. These attributes are used mainly in the Prerequisites section of the various procedures (in the other PRs that you are reviewing/merging as part of this effort) for deploying to OpenShift.
For example, in the Prerequisite, "You have OpenJDK {jdk-version-other} or later installed."
Per your request, I will update you via email with more details. Again, many thanks for your reviews and feedback.
Status for workflow
|
This PR aims to update the upstream _attributes.adoc file with the additional attributes needed to migrate the Red Hat Deploying to OpenShift Container Platform guide to the upstream Quarkus community and to facilitate the single-sourcing of the guide back downstream.
For more information, see QDOCS-1082