-
Notifications
You must be signed in to change notification settings - Fork 103
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
Enable revive's defer rule in golangci-lint #2070
Conversation
3330c71
to
7f64d37
Compare
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) | ||
defer cancel() | ||
cmd, err := browserLauncher(ctx, actionUri) | ||
cmd, err := browserLauncher(context.TODO(), actionUri) |
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.
Can't move a timeout with a defer cancel
to a new function because then calling cmd.Start
at the end of that function makes it fail immediately. I don't think the timeout is super meaningful here so I've removed it rather than adding a lint exception.
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.
nice!
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.
Nice, this looks like a pretty solid improvement.
Enables https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#defer in our ruleset, and fixes existing violations.