Skip to content

Bump golangci to v1.39.0 #4241

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

Closed
wants to merge 1 commit into from
Closed

Bump golangci to v1.39.0 #4241

wants to merge 1 commit into from

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Jun 1, 2021

Signed-off-by: Gábor Lipták [email protected]

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@pstibrany
Copy link
Contributor

Hello. I've built a new build-image with your change: quay.io/cortexproject/build-image:bump-golangci-pr-4241-935bae54c.

Please follow instructions at https://cortexmetrics.io/docs/contributing/how-to-update-the-build-image/ (step 1 and 2 are done) and update this PR with this new image. Thank you!

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Jun 2, 2021
@gliptak
Copy link
Contributor Author

gliptak commented Jun 2, 2021

@pstibrany thank you for pointing to the update process. Made the update

@pstibrany
Copy link
Contributor

pstibrany commented Jun 2, 2021

@pstibrany thank you for pointing to the update process. Made the update

Thank you. There are now lint errors to be fixed: https://github.com/cortexproject/cortex/pull/4241/checks?check_run_id=2727067541

Do you also want to enable promlinter (in file /.golangci.yml) as discussed in #4171?

@gliptak
Copy link
Contributor Author

gliptak commented Jun 2, 2021

@pstibrany I'm working to setup a build locally and target to correct lint errors. I propose to have promlinter as followup PR

@pstibrany
Copy link
Contributor

@pstibrany I'm working to setup a build locally and target to correct lint errors. I propose to have promlinter as followup PR

Thank you. Followup PR for promlinter sounds good.

@gliptak
Copy link
Contributor Author

gliptak commented Jun 2, 2021

I'm failing make lint (running in golang:1.16 Docker image)

# make lint

>>>> Entering build container: protos
make: Entering directory '/go/src/github.com/cortexproject/cortex'
make: *** No rule to make target 'protos'.  Stop.
make: Leaving directory '/go/src/github.com/cortexproject/cortex'
Command exited with non-zero status 2
0.01user 0.04system 0:00.82elapsed 7%CPU (0avgtext+0avgdata 64204maxresi
dent)k
0inputs+0outputs (0major+5293minor)pagefaults 0swaps
make: *** [Makefile:134: protos] Error 2

for reference I ran apt install docker.io sudo time protobuf-compiler protobuf-c-compiler

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 3, 2021
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lint is still failing. Could you take a look and fix warning too, please?

level=warning msg="[runner] The linter 'golint' is deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner.  Replaced by revive."
level=warning msg="[runner/nolint] Found unknown linters in //nolint directives: golint ignore stutter warning"
pkg/util/validation/errors.go:14:6: type name will be used as validation.ValidationError by other packages, and that stutters; consider calling this Error (golint)
type ValidationError error // nolint: golint ignore stutter warning

Also, I've just merged #4244. Could you update the build-image tag in Makefile as well, please?

@gliptak
Copy link
Contributor Author

gliptak commented Jun 3, 2021

level=warning msg="[runner] The linter 'golint' is deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner.  Replaced by revive."

seems to be a larger update (in particular related to incorrect comments in the codebase)
golangci/golangci-lint#1892

As for

level=warning msg="[runner/nolint] Found unknown linters in //nolint directives: golint ignore stutter warning"
pkg/util/validation/errors.go:14:6: type name will be used as validation.ValidationError by other packages, and that stutters; consider calling this Error (golint)
type ValidationError error // nolint: golint ignore stutter warning

I tried various formats without success. Related to golangci/golangci-lint#1450

Might consider going to 1.139.0 first? Thoughts?

@pracucci
Copy link
Contributor

pracucci commented Jun 8, 2021

Might consider going to 1.139.0 first?

Sounds good to me. Would allow us to do it step-by-step.

@gliptak
Copy link
Contributor Author

gliptak commented Jun 8, 2021

@pracucci please publish build-image

@pstibrany
Copy link
Contributor

@pracucci please publish build-image

Doing this right now.

@pstibrany
Copy link
Contributor

New image: quay.io/cortexproject/build-image:golangci-update-pr-4241-40550805d

@gliptak gliptak changed the title Bump golangci to v1.40.1 Bump golangci to v1.39.0 Jun 9, 2021
Signed-off-by: Gábor Lipták <[email protected]>
@pracucci
Copy link
Contributor

@gliptak Could you take a look at the failing linter, please?

@gliptak gliptak closed this Jun 22, 2021
@gliptak gliptak deleted the patch-1 branch June 22, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants