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

Bump golangci lint #2400

Closed

Conversation

BronzeDeer
Copy link
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
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
Collaborator

#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
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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants