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

Updating Adding Tools section in EXTENDING_DOCKER.adoc #607

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Ajaypathak372
Copy link

This is a simple demo for how to add tools in jenkinsfile-runner. Same can be applied to add other tools.

Comment on lines +71 to +77
----
tool:
git:
installations:
- name: git
home: /usr/bin/git
----
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe it would be better to inform the user of the perspective you are coming from throughout, so they could avoid the pitfall wherein they follow your instructions as is and would likely be getting errors due to setup config issues. For example in here, you could add that say for macOS users who would ordinarily install their git using Homebrew or brew, their git path would alternatively or additionally be something like /opt/homebrew/bin/git. As is the code would not work for some macOS and Windows users.

Copy link
Author

Choose a reason for hiding this comment

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

Here we are installing git in jenkinsfile-runner docker image which is linux based, so I think there is no need to specify the installation paths for macOS or windows users.

Copy link
Member

Choose a reason for hiding this comment

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

You will have to explain at least something like "the jenkinsfile-runner docker image is linux-based", or people will not be aware of it.

Copy link
Author

Choose a reason for hiding this comment

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

I think the one who knows jenkins, can easily understand this, and also isn't it obvious that it is Linux based docker image for anyone using jenkinsfile-runner docker image?

Copy link
Member

Choose a reason for hiding this comment

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

Well we have to consider new-comers to Jenkins as well like yourself.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, you are right but see all docker images created from base images like ubuntu, centos and these are linux based and since here this is the Docker doc for jenkinsfile-runner, so I guess there is no need to mention anything like here.

home: /usr/bin/git
----

Now you can run a Jenkinsfile having `git` as a stage like given below
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to state what is the intention of this Jenkinsfile content, since some contributors will inevitably be newbies. You can for example say something like "The following is to illustrate a 'Hello, World' example by way of a Jenksinsfile...". Otherwise it would take time for the user of this guide to guess the context of this bit of documentation.

Copy link
Author

Choose a reason for hiding this comment

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

Any update about this PR?

----
docker run --rm -v $(pwd)/Jenkinsfile:/workspace/Jenkinsfile -v $(pwd)/jenkins.yaml:/usr/share/jenkins/ref/casc/jenkins.yaml jenkinsfile-runner:my-image
----
Here `jenkinsfile-runner:my-image` is custom docker image created from above Dockerfile.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of concluding with this line, you could extend to other cases by generalising or providing a list of the names of the other tools (as we have discussed on Gitter before), which they could try using the same procedure. Otherwise it appears like this is only useful for the git tool alone. With more details the user of the documentation would not feel lost as the stud may appear incomplete to some who would like to learn more. It would indeed be a very nice documentation if the desired effect could be described in detail, perhaps with screenshots to capture the intended outcome.

}
}
----
You can then run `jenkinsfile-runner` like this
Copy link
Member

Choose a reason for hiding this comment

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

Better use more formal language like "You can then run jenkinsfile-runner like the following:" or "You can then run jenkinsfile-runner like what is shown below:" then using the more colloquial phrase "like this" as in casual conversation. Using a colon would definitely help with the punctuation.

@oleg-nenashev oleg-nenashev marked this pull request as ready for review March 8, 2022 12:02
@oleg-nenashev oleg-nenashev requested a review from a team as a code owner March 8, 2022 12:02
@oleg-nenashev oleg-nenashev added the documentation Documentation updates label May 23, 2022
@oleg-nenashev oleg-nenashev self-requested a review June 13, 2022 12:04
@oleg-nenashev
Copy link
Member

Sorry I definitely missed the last update

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

I will need to reword it before merging. Installing tools from the internet should be discouraged in Jenkinsfile Runner. Tools should be baked into the images whenever possible

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

Successfully merging this pull request may close these issues.

None yet

3 participants