feat: support params for pipelineruns and taskruns #23
Conversation
tekton-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
size/XL
| } | ||
|
|
||
| paramStr := paramsArg[0] | ||
| if paramStr == "" { |
There was a problem hiding this comment.
I think this check is not necessary, since you are checking the value again in ParsePipelineRunParams function
| value := kv[1] | ||
|
|
||
| param, err := parseParamValue(key, value) | ||
| if err != nil { |
There was a problem hiding this comment.
Is it wise to recreate parameter when there was an error during parsing?
| paramDescs := make([]string, 0, len(pipelineParams)) | ||
| for _, p := range pipelineParams { | ||
| var valueStr string | ||
| switch p.Value.Type { |
There was a problem hiding this comment.
This switch statement is the same one as in handlerStartTask function. Can it extract to a standalone function?
| @@ -0,0 +1,71 @@ | |||
| package tools | |||
There was a problem hiding this comment.
I would vote for introducing ginkgo testing framework (should not be part of this PR)
tekton-robot
added
the
needs-rebase
|
@waveywaves: PR needs rebase. 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. |
Changes
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
Release Notes