-
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: Create procedure for Deploying on OpenShift using Docker build strategy #45918
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. |
ea3166e
to
d431deb
Compare
d431deb
to
54134b5
Compare
54134b5
to
b87770d
Compare
This comment has been minimized.
This comment has been minimized.
🙈 The PR is closed and the preview is expired. |
Hi @iocanel, |
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 changed some items to bullet points to better reflect that they are a collection of independent options rather than a sequence of steps. Additionally, to make it clearer that these options are optional, I replaced the imperative verb form with the infinitive "To ..." form. Feel free to apply this approach to any other PRs that include similar lists of optional steps.
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
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 Sheila,
Great work! With your agreement, I’ve taken the opportunity to provide a more thorough review. I hope you find it helpful. Feel free to apply or disregard any suggestions as you see fit.
Thanks!
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
oc logs -f _openshift-helloworld-1-gzzrx_ | ||
---- | ||
+ | ||
[source,shell,subs=attributes+] |
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.
[source,shell,subs=attributes+] | |
.Example output | |
[source,shell,subs=attributes+] |
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'm unsure whether we want to label outputs, but if we do, we should aim to apply it consistently across all upstream-downstream documentation. Something to think about for the future.
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.
Good point and +1. I think it looks better to have the label, but for consistency reasons, I might leave this as it is for now and look to deciding whether or not to apply it in the future across the other guides.
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
docs/src/main/asciidoc/deploying-to-openshift-docker-howto.adoc
Outdated
Show resolved
Hide resolved
b87770d
to
82d148a
Compare
This comment has been minimized.
This comment has been minimized.
82d148a
to
97bc55a
Compare
:topics: devops,kubernetes,openshift,cloud,deployment | ||
:extensions: io.quarkus:quarkus-openshift | ||
|
||
As an application developer, you can deploy your applications to {openshift} by using the Docker build strategy as a deployment option. |
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.
Maybe add a quick explanation on what the docker build strategy
is.
I think that we need to let them know that its actually a docker build taking place inside openshift using the artifact that was build locally. In other words a more customizable version of the binary build that is the default option.
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.
@iocanel, I have completed all required updates from SME and peer reviews. Can you please advise if this is ready to merge?
This comment has been minimized.
This comment has been minimized.
97bc55a
to
0544782
Compare
This comment has been minimized.
This comment has been minimized.
0544782
to
4bd60da
Compare
This comment has been minimized.
This comment has been minimized.
Signed-off-by: shjones <[email protected]>
4bd60da
to
c947b4d
Compare
I squashed the changes. |
@iocanel can you have a final look at this one? |
Status for workflow
|
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
This PR aims to draft a procedure for the following use case
Reference: Based on the Red Hat build of Quarkus guide Deploying on OpenShift Container Platform