Skip to content
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

[27.x backport] update golangci-lint to v1.62.0 #5640

Conversation

austinvazquez
Copy link
Contributor

- What I did

Backports #5632 to 27.x

full diff: golangci/golangci-lint@v1.61.0...v1.62.0
Changelog: https://golangci-lint.run/product/changelog/#v1620

(cherry picked from commit 07e5ddd)

- Description for the changelog

n/a

- A picture of a cute animal (not mandatory but encouraged)

full diff: golangci/golangci-lint@v1.61.0...v1.62.0
Changelog: https://golangci-lint.run/product/changelog/#v1620

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 07e5ddd)
Signed-off-by: Austin Vazquez <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.54%. Comparing base (6b9fb59) to head (47a4f70).
Report is 3 commits behind head on 27.x.

Additional details and impacted files
@@           Coverage Diff           @@
##             27.x    #5640   +/-   ##
=======================================
  Coverage   58.54%   58.54%           
=======================================
  Files         346      346           
  Lines       29335    29335           
=======================================
  Hits        17173    17173           
  Misses      11184    11184           
  Partials      978      978           
---- 🚨 Try these New Features:

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah merged commit ce03e25 into docker:27.x Nov 22, 2024
96 checks passed
@thaJeztah
Copy link
Member

@austinvazquez I should probably mention as "long lived" branches can be relevant to you. The reason I'm trying to keep a bit "up" with golangci-lint is that older versions are not always compatible with updated versions of Go. So whenever I update versions of Go, it's usually also a reminder "let me check if there's updates for Golang CI lint to go in first". Some of the GolangCI-lint updates are easy ('just update and be done!') but you may happen to run into updated linters, now expecting you to make changes in code

I guess that last situation depends a bit; if the branch is expected to still live long enough, it may be worth going the extra mile to include the linting fixes. If the branch is not expected to live much longer and those fixes are "complicated" it's of course a matter of outweighing the pros/cons (are they actually relevant linting fixes, or just "make the code slightly better"?); disabling the linter (or a global "ignore") is always an option for those of course.

@thaJeztah
Copy link
Member

So maybe some of these could be relevant for "your" branches (I realised I didn't mark all of them for cherry-pick), but as described above "it depends" to what extent to go and fix all linting issues. 😅

@austinvazquez austinvazquez deleted the cherry-pick-07e5ddd05441340dd044c14019dca22fe024a111-to-27.x branch November 22, 2024 17:33
@austinvazquez
Copy link
Contributor Author

@thaJeztah, thanks for the callout. Yeah there is certainly a balance to be maintained. Sometimes I just enjoy the grunt work as a break from more taxing problems, but I also understand the value just might not be worth the effort or risk for that matter. The Go updates are nice to validate toolchain updates are not breaking functionality. They have the Go promise but "trust but verify".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants