-
Notifications
You must be signed in to change notification settings - Fork 1.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
Improve documentation around variable behaviour and gotchas #3636
Conversation
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.
Thanks,
this looks good.
/lgtm
docs/tasks.md
Outdated
Tekton provides variables to inject values into the contents of certain fields. | ||
The values you can inject come from a range of sources including other fields | ||
in the Task or Pipeline, context-sensitive information that Tekton provides, | ||
and runtime information received from a TaskRun or PipelineRun. | ||
|
||
The mechanism of variable substitution is quite simple - string replacement is | ||
performed by the Tekton Controller when a TaskRun or PipelineRun is executed. | ||
|
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.
it would be helpful to have this in pipelines docs because it's just referencing the variables.md
as well, what do you think?
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.
Yeah, that makes sense, I'll update this PR to add details to the pipelines doc too.
We used to link from the docs directory's README.md to our variables.md doc but variables.md doesn't really explain the way variables work - it just describes what values they can be used for and where they can be used in our CRDs. This commit changes the docs directory's README.md to point at the variable documentation in our task doc, and adds a few lines there to talk a bit more about how the substitution happens. This commit also describes how to safely inject variables into `bash` `script` blocks in Tasks without accidentally eval'ing the variable contents.
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.
thanks @sbwsg!
/meow
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jerop The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Changes
We used to link from the docs directory's README.md to our variables.md doc
but variables.md doesn't really explain the way variables work - it just
describes what values they can be used for and where they can be used in
our CRDs.
This commit changes the docs directory's README.md to point at the variable
documentation in our task doc, and adds a few lines there to talk a bit
more about how the substitution happens.
This commit also describes how to safely inject variables into
bash
script
blocks in Tasks without accidentally eval'ing the variablecontents.
Closes #3039
Closes #3458
/kind documentation
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Release Notes