-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat: add IN and NOTIN to step condition operators #129
Conversation
engine/step/condition.go
Outdated
func matchList(valStr, expStr string) bool { | ||
values := strings.Split(expStr, ",") | ||
for _, v := range values { | ||
if valStr == v { |
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'm always paranoid about whitespaces messing up comparisons.
Should we TrimSpace(v) ?
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.
If complexity is a problem, we could have a Prepare() step on resolution load
f1d58d1
to
76df58d
Compare
conditions: | ||
- type: check | ||
if: | ||
- value: '{{.step.this.output.items}}' |
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.
the condition is backwards, isnt it?
foo IS IN foo,bar
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.
Indeed, it doesn't match the code. Fixing.
Looks good overall, 2 issues to address imo:
|
338b8bc
to
e57e9a6
Compare
c5004f2
to
2225921
Compare
2225921
to
385e7df
Compare
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
385e7df
to
0f094f0
Compare
CDS Report test#600.0 ✘
|
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 for the feature.
Missing: patch of template-schema.json to include the new feature: https://github.com/ovh/utask/blob/master/hack/template-schema.json#L656
Test doesn't pass.
CDS Report test#648.0 ✘
|
These operators allows to check that a value is present or not in a list of values. The allowed/disabled values are represented as a comma-separated string. Signed-off-by: William Poussier <william.poussier@gmail.com> Co-Authored-By: Thomas Schaffer <thomas.schaffer@corp.ovh.com>
Signed-off-by: Thomas Schaffer <thomas.schaffer@corp.ovh.com>
923e723
to
6c959c7
Compare
Signed-off-by: William Poussier <william.poussier@gmail.com>
These operators allows to check that a value is present or not
in a list of values. The allowed/disabled values are represented
as a comma-separated string.
Feature.
A value cannot be checked against a list of potential candidates in a step condition.
This permits to assert that a value is present or not in a list of acceptable values.
No.