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

Jenkins.io - Updating Docker installation guide #6512

Merged

Conversation

kmartens27
Copy link
Contributor

This documentation is being updated as a part of the Java 17 transition.

The Docker installation guide does not contain any specific Java version text, but I have gone through to update some of the syntax and the page formatting. Previously, the text was composed with a line limit for text, but has been updated to be more in line with Asciidoc best practices.

This is for the Docker installation and Docker installation instruction pages. The later is an include file that is part of the overall docker.adoc page.

Most of the existing text was left as is, so there is room for further improvement/updates

@kmartens27 kmartens27 requested a review from a team as a code owner July 5, 2023 21:31
@probot-autolabeler probot-autolabeler bot added the documentation Jenkins documentation, including user and developer docs, solution pages, etc. label Jul 5, 2023
Copy link
Contributor

@LizGaudet LizGaudet left a comment

Choose a reason for hiding this comment

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

I updated the content even though you did not request that. Feel free to use or not.

content/doc/book/installing/_docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/_docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/_docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/_docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/_docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/docker.adoc Outdated Show resolved Hide resolved
content/doc/book/installing/docker.adoc Outdated Show resolved Hide resolved
@@ -52,15 +52,15 @@ Due to the use of a privileged container, this is recommended, though it require
This environment variable controls the root directory where Docker TLS certificates are managed.
<8> Maps the `/certs/client` directory inside the container to a Docker volume named `jenkins-docker-certs` as created above.
<9> Maps the `/var/jenkins_home` directory inside the container to the Docker volume named `jenkins-data`.
This will allow for other Docker containers controlled by this Docker container's Docker daemon to mount data from Jenkins.
This allows for other Docker containers controlled by this Docker container's Docker daemon to mount data from Jenkins.
Copy link
Contributor

Choose a reason for hiding this comment

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

Phrasing seems odd to me with the "for" in the phrase. However, my English grammar is weak, so I am perfectly fine if you correct me and tell me that it is more correct to say "allow for other" than to say "allows other". Optional.

Suggested change
This allows for other Docker containers controlled by this Docker container's Docker daemon to mount data from Jenkins.
This allows other Docker containers controlled by this Docker container's Docker daemon to mount data from Jenkins.

Copy link
Contributor

Choose a reason for hiding this comment

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

My English grammar is super poor, but I've got a few tools to help.
One of them got rid of the for. 🤷

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Needs one or two additional changes in order to actually use Java 17. Changes submitted in 👍

  • 182d779 - Use Java 17 base container images
  • 3aa1041 - Use lts-slim-jdk17 in scaling kubernetes chapter

Would have preferred to use the symbolic version {jenkins-stable} but it
is not replaced with the actual version when I run the site build
locally.  Better to have a short term reference to jdk17 without the
precise version than to continue using the jdk11 version.
@MarkEWaite MarkEWaite merged commit c7ba6be into jenkins-infra:master Jul 7, 2023
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Jenkins documentation, including user and developer docs, solution pages, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants