Templatize describe Pipeline #131
Conversation
tekton-robot
added
size/L
| @@ -91,11 +91,11 @@ func TestPipelinesDescribe_with_run(t *testing.T) { | |||
| } | |||
|
|
|||
| expected := []string{ | |||
| "Name: pipeline", | |||
| "Name: pipeline", | |||
There was a problem hiding this comment.
Fixed to align with describe pipelinerun!
https://github.com/tektoncd/cli/blob/master/pkg/cmd/pipelinerun/describe.go#L33
| "Tasks", | ||
| "No tasks found\n", | ||
| "No tasks\n", |
There was a problem hiding this comment.
The same with the comment. Since describe pipelinerun says No resources, I remove found to print the same message.
https://github.com/tektoncd/cli/blob/master/pkg/cmd/pipelinerun/describe.go#L52
|
/ok-to-test |
tekton-robot
removed
the
needs-ok-to-test
pkg/cmd/pipeline/describe.go
Outdated
| if len(runs.Items) == 0 { | ||
| fmt.Fprintln(w, "No runs found") | ||
| return | ||
| panic(err) |
There was a problem hiding this comment.
Any reason for panic instead of returning an error? 🤔
There was a problem hiding this comment.
There are 2 places where it panics
template.Must- within
template.Executesee: errRecover
But errRecover does return/set the err in some cases, where we could just exit by writing StdErr (use cli.Stream)
There was a problem hiding this comment.
Thanks for the review! I fixed to simply return an error instead of panicing!
|
/lgtm @sthaha one more review round? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shuheiktgw, 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 |
tekton-robot
added
the
approved
Changes
I've changed to use template for describing a pipeline! Thanks! 👍
Related Issues
Related PRs
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide
for more details.