-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
deps: bump various opentelemetry modules and golangci-lint #7175
Conversation
a8f6ffe
to
cca332f
Compare
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
db988ec
to
1955470
Compare
1955470
to
c2e8efb
Compare
c2e8efb
to
2a57f71
Compare
@@ -1,6 +1,9 @@ | |||
run: | |||
timeout: 5m | |||
|
|||
issues: | |||
max-same-issues: 0 # don't hide issues in CI runs because they are the same type |
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.
This is a small quality-of-life improvement, I hope: Previously, if the linter found a total of 5 issues, but they all had the same type, it wouldn't show you all of them. Now, we ask it to just spit it all out, all at once.
@@ -28,7 +28,7 @@ ifeq ($(WASM_ENABLED),1) | |||
GO_TAGS = -tags=opa_wasm | |||
endif | |||
|
|||
GOLANGCI_LINT_VERSION := v1.59.1 | |||
GOLANGCI_LINT_VERSION := v1.60.1 |
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.
That's the aforementioned middle-ground: It works with our go version, but doesn't bring in too many new rules.
@@ -392,7 +392,7 @@ func eval(args []string, params evalCommandParams, w io.Writer) (bool, error) { | |||
for _, t := range timers { | |||
val, ok := t[name].(int64) | |||
if !ok { | |||
return false, fmt.Errorf("missing timer for %s" + name) |
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 not all bad! this is a good find
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.
Thanks for working on this @srenatus!
7419638
to
39e9f68
Compare
This release had a fix to avoid "superfluous call to WriteHeader" log lines: https://github.com/open-telemetry/opentelemetry-go-contrib/releases/tag/v1.30.0 And there are further fixes in the releases we hadn't pulled in. Feels like a good idea to update this once in a while. Signed-off-by: Stephan Renatus <[email protected]>
The previous version has been failing without any good reason for me, so let's try this. About the version pick: It's not the latest version (v1.62.0 at the moment), because that would introduce a new revive rule, redeclares-builtin-id, and that flags every variable called `min` or `max` in the code base. I had started addressing these, but they were just too many. The new issues related to this version are mostly that it complains whenever it finds a non-static string that makes its way into a printf- like function. However, that's a common pattern in some place here, so I've sprinkled some nolint:govet on it. Signed-off-by: Stephan Renatus <[email protected]>
39e9f68
to
bf91031
Compare
This release had a fix to avoid "superfluous call to WriteHeader" log lines:
https://github.com/open-telemetry/opentelemetry-go-contrib/releases/tag/v1.30.0
And there are further fixes in the releases we hadn't pulled in. Feels like a good idea to update this once in a while.
Re: Golangci-lint
The previous version has been failing without any good reason for me,
so let's try this one.
About the version pick: It's not the latest version (v1.62.0 at the
moment), because that would introduce a new revive rule,
redeclares-builtin-id, and that flags every variable called
min
ormax
in the code base. I had started addressing these, but they werejust too many. Also I hadn't found a working way to disable said rule.
The new issues related to this version are mostly that it complains
whenever it finds a non-static string that makes its way into a printf-
like function. However, that's a common pattern in some place here, so
I've sprinkled some
//nolint:govet
on it.When this goes in, please rebase, to keep the two commits separate.