-
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
Add example and docs for array param with defaults 📜 #4518
Add example and docs for array param with defaults 📜 #4518
Conversation
8de5615
to
442cca5
Compare
docs/tasks.md
Outdated
|
||
```yaml | ||
params: | ||
- name: flags |
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.
This example is a bit confusing; I would expect most people to want to use only one parameter for flags and to supply some default values for that parameter (am I misunderstanding the example?). Also, could you provide slightly more realistic default values for flags? e.g. the example above has "arg1=foo" and "--randomflag".
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 - ill update this
docs/tasks.md
Outdated
@@ -502,6 +502,23 @@ spec: | |||
value: "http://google.com" | |||
``` | |||
|
|||
Parameter declarations include default values which will be used if the parameter is not specified, for example to |
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.
Is this sentence talking about default values for string and array parameters that are provided by Tekton or supplied by the user?
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.
supplied by the author of the Task/Pipeline - ill add a bit more text to try to make it more clear
default: "baz" | ||
steps: | ||
- name: build | ||
image: bash:3.2 |
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.
Nit: IMO, adding a comment that documents the fully-resolved echo
that will be executed by the task post-parameter-resolution will clarify the example.
@bobcatfish it looks like this might not make it for v0.33.0 - is that ok for you? |
While working on (finally) updating the array result and object param/result TEPs (tektoncd/community#479 tektoncd/community#477) I realized I hadn't included an example of how to specify defaults for the new format, so I looked for an example of how we currently do this for arrays, but we had none! Hopefully now we do :D
442cca5
to
fee9d2c
Compare
hm seems like something went pretty dramatically wrong, many tests failing with:
/test pull-tekton-pipeline-alpha-integration-tests |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jerop, vdemeester 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 |
/test pull-tekton-pipeline-alpha-integration-tests |
/test pull-tekton-pipeline-integration-tests |
Changes
While working on (finally) updating the array result and object
param/result TEPs (tektoncd/community#479
tektoncd/community#477) I realized I hadn't
included an example of how to specify defaults for the new format, so I
looked for an example of how we currently do this for arrays, but we had
none! Hopefully now we do :D
/kind documentation
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes