-
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
updating an existing example pipelinerun with array results #6055
updating an existing example pipelinerun with array results #6055
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.
@pritidesai do you mind passing array results to matrix and when expressions in this example, in addition to params? for the matrix one to work, #6056 has to be merged first
#6056 has updated one example to use matrix and results. Maybe it's ok to let this PR's example only focusing on how to use array results? |
Thank you @jerop and @Yongxuanzhang for the review! Array indexing into matrix is not yet supported, I agree with @Yongxuanzhang to delay adding an example as it's not possible to run it as part of our CI when the feature is not implemented yet. I prefer to create a separate PR for referencing array results into when expressions with a necessary doc update. A separate PR will be focused on just /test check-pr-has-kind-label |
@pritidesai: The specified target(s) for
The following commands are available to trigger optional jobs:
Use 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. |
sounds good to me to add those examples in other PRs, just want to make sure that those cases are covered too |
Yup, added a note in the issue which is tracking a list of action items for the promotion, thanks! |
/retest |
diff=$(diff <(printf "%s\n" "${VALUE[@]}") <(printf "%s\n" "${EXPECTED[@]}")) | ||
if [[ -z "$diff" ]]; then | ||
echo "Get expected: ${VALUE}" | ||
echo "Got expected: ${VALUE}" |
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.
I think we may need to remove the exit 0 here. Otherwise the next validation won't happen
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.
yup, you are absolutely right 💯 Thanks for pointing that out 👍 Dropped exit 0
from here.
efebac2
to
8c88de2
Compare
Updating a PR to validate the length of the array results including each element in the array. Also, added an example to validate an array when one of the elements is empty string. Signed-off-by: pritidesai <pdesai@us.ibm.com>
8c88de2
to
d945634
Compare
/lgtm |
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.
Thank you @pritidesai
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afrittoli 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 |
Changes
Updating a PR to validate the length of the array results including each element in the array.
Also, added an example to validate an array when one of the elements is empty string.
This is for the preparation of a potential promotion of #5688.
Signed-off-by: pritidesai pdesai@us.ibm.com
/kind misc
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes