-
Notifications
You must be signed in to change notification settings - Fork 223
golangci-lint: disable noctx linter #2485
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
Conversation
commit 2d75d8e bumped the linter version without fixing the new issues. Follow-up for containers#2482 Signed-off-by: Giuseppe Scrivano <[email protected]>
Reviewer's GuideThis PR updates the GolangCI-Lint configuration by disabling the “noctx” linter rule to accommodate existing context-related code that wasn’t updated after bumping the linter version. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Luap99
left a comment
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.
LGTM, looking at the other PR I just adds pointless boilerplate, context.Background() would behave no different that the current code so I think the simpler solution to disable the lint is better.
Using a context would only make sense if a external caller would pass in a context that can be cancelled
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.
|
@mheon PTAL |
kolyshkin
left a comment
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.
In theory, we could just disable new noctx linters (ones for net.Dial, exec.Command etc). Practically, noctx is not configurable, so I guess disabling it is the best way forward.
LGTM.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, kolyshkin, Luap99, sourcery-ai[bot] The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
commit 2d75d8e bumped the linter version without fixing the new issues.
Follow-up for #2482
Alternative to #2484
Summary by Sourcery
CI: