Validate Task for undefined variables #412
Conversation
knative-prow-robot
added
the
size/L
There was a problem hiding this comment.
Great work on this PR. I noticed in this file there is no validation of the expected error. There are tests for invalid cases which also include the expected error. I like that practice more because it helps developer understand
- the expected error for that case
- when there is change in code about returned error the test fails
- test is failing for right reasons
A Task can fail at "runtime", when the variables are interpolated based on resources and parameters. Adding validation for those inexistent variable shorten the loop when trying to write a Pipeline. Signed-off-by: Vincent Demeester <[email protected]>
vdemeester
force-pushed
the
212-task-pipeline-variable-validation
branch
from
4b6b880 to
8a10953
Compare
|
Thanks for adding this @vdemeester !!! 😻 (ill leave it for @shashwathi to sign off on this when she's happy :)) |
|
/assign @shashwathi |
Signed-off-by: Vincent Demeester <[email protected]>
|
The following is the coverage report on pkg/.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shashwathi, 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 |
knative-prow-robot
added
the
approved
A Task can fail at "runtime", when the variables are interpolated
based on resources and parameters. Adding validation for those
inexistent variable shorten the loop when trying to write a Pipeline.
Fixes #212
cc @bobcatfish @tanner-bruce
Signed-off-by: Vincent Demeester [email protected]