Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

Bump golangci lint#2400

Closed
BronzeDeer wants to merge 3 commits intoGoogleContainerTools:mainfrom
BronzeDeer:bump-golangci-lint
Closed

Bump golangci lint#2400
BronzeDeer wants to merge 3 commits intoGoogleContainerTools:mainfrom
BronzeDeer:bump-golangci-lint

Conversation

@BronzeDeer
Copy link
Copy Markdown
Contributor

Description

As part of #2384 it was discovered that the older golangci-lint version does not support go 1.20. This PR bumps golangci-lint to the newest release (1.51.1) and fixes all newly discoverd gofmt, goimport and linting errors

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed.

No new features were added, so no tests needed to be written

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

- Adapted error comparison according to linter recommendation
- Disabled noctx linting for http request where canceling makes no sense
- Disabled nilerror linting where nil error is returned on purpose
- Disabled makezero linter where slice is explicitly deepcopied
@BronzeDeer BronzeDeer force-pushed the bump-golangci-lint branch from 84b13c4 to 92ce78c Compare March 1, 2023 13:20
@BronzeDeer
Copy link
Copy Markdown
Contributor Author

Fixed an issue in which gofmt would reformat the boilerplate if there was no empty line between it and the package declaration. If gofmt reformats the blockcomment of the boilerplate, the boilerplate detection fails. The fix gofmt commit now adds the blank lines for consistency across the whole codebase, a more durable fix would likely be to make the boilerplate detection also ignore leading and trailing whitespace changes in the boilerplate

@BronzeDeer BronzeDeer mentioned this pull request Mar 10, 2023
4 tasks
@imjasonh
Copy link
Copy Markdown
Contributor

#2425 is merged, so this should no longer be needed right? (Closing; let me know if I should reopen)

@imjasonh imjasonh closed this Mar 21, 2023
@BronzeDeer
Copy link
Copy Markdown
Contributor Author

#2425 is merged, so this should no longer be needed right? (Closing; let me know if I should reopen)

Correct, since #2425 was based ontop of this, it merged it into main, although due to the squash this is not immediately visible

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants