-
Notifications
You must be signed in to change notification settings - Fork 1.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
✨ Enable linters: asasalint, bidichk, durationcheck, errchkjson. Fix findings #7208
Conversation
@@ -42,7 +42,7 @@ func (m matchConditions) Match(actual interface{}) (success bool, err error) { | |||
elems = append(elems, MatchCondition(condition)) | |||
} | |||
|
|||
return gomega.ConsistOf(elems).Match(actual) | |||
return gomega.ConsistOf(elems...).Match(actual) |
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.
Wondering why this was not failing
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.
It's explicitly supported:
You typically pass variadic arguments to ConsistOf (as in the examples above). However, if you need to pass in a slice you can provided that it is the only element passed in to ConsistOf:
//ConsistOf succeeds if actual contains precisely the elements passed into the matcher. The ordering of the elements does not matter.
//By default ConsistOf() uses Equal() to match the elements, however custom matchers can be passed in instead. Here are some examples:
//
// Expect([]string{"Foo", "FooBar"}).Should(ConsistOf("FooBar", "Foo"))
// Expect([]string{"Foo", "FooBar"}).Should(ConsistOf(ContainSubstring("Bar"), "Foo"))
// Expect([]string{"Foo", "FooBar"}).Should(ConsistOf(ContainSubstring("Foo"), ContainSubstring("Foo")))
//
//Actual must be an array, slice or map. For maps, ConsistOf matches against the map's values.
//
//You typically pass variadic arguments to ConsistOf (as in the examples above). However, if you need to pass in a slice you can provided that it
//is the only element passed in to ConsistOf:
//
// Expect([]string{"Foo", "FooBar"}).Should(ConsistOf([]string{"FooBar", "Foo"}))
//
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 guess that's something that you can only do if you have ... interface{}
as parameter and handle it accordingly.
So that's more like a false positive. But as both are valid I'm +1 to the change
I'm generally +1 for adding these linters, but let's wait for some more opinions |
/lgtm +1 from me as well |
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.
+1 to adding these
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
What this PR does / why we need it:
Enables some of the "bug" finding linters in golangci-lint that are currently disabled.
https://golangci-lint.run/usage/linters/
Also fixes some findings (including Hadolint findings).
There are some bug linters that find alot more. But I will add these 1 by 1 since they require more work to review.