-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
add GetElementByKey #3044
add GetElementByKey #3044
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Shell32-Natsu 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 |
/cc @monopole |
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
3f32eec
to
8c67f8c
Compare
8c67f8c
to
2ba6066
Compare
kyaml/yaml/fns.go
Outdated
// NoValue indicates that matcher should only consider the key and ignore | ||
// the actual value in the list. FieldValue must be empty when NoValue is | ||
// set to true. | ||
NoValue bool `yaml:"noValue,omitempty"` |
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 field name MatchAnyValue
would be clearer here.
It's like using a wildcard *
in the value.
Explicitly supporting wildcards or more general regexps could be the next step.
Any opinion on adding MatchAnyField
?
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.
Yeah, MatchAnyValue
is better.
kyaml/yaml/fns.go
Outdated
func GetElementByKey(key string) ElementMatcher { | ||
return ElementMatcher{FieldName: key, NoValue: true} | ||
} | ||
|
||
// ElementMatcher returns the first element from a Sequence matching the | ||
// specified field's 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.
// If there's no match, and no configuration error, the matcher returns nil, nil.
kyaml/yaml/fns_test.go
Outdated
|
||
rn, err = node.Pipe(ElementMatcher{FieldName: "a"}) | ||
assert.NoError(t, err) | ||
assert.Equal(t, "", assertNoErrorString(t)(rn.String())) |
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.
L92 is meant to demonstrate a match failure for fieldValue "".
The documentation for type ElementMatcher struct
doesn't actually specify
what is returned if there's no match- it's nil, but we should say so up above.
So L92 is technically correct, since (nil *RNode).String returns "" ,
but lets make it just
assert.Nil(t, rn)
assert.Equal(t, "f: g\n", assertNoErrorString(t)(rn.String())) | ||
} | ||
|
||
func TestElementMatcherWithNoValue(t *testing.T) { |
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.
BTW, thanks for adding this. It covers all the bases (that's a baseball idiom).
5a36f4f
to
bb60c29
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
for #3038