-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
*: fix revive linter #16634
*: fix revive linter #16634
Conversation
Remove old revive_pass in the bash scripts and migirate the revive.toml into golangci linter_settings. Signed-off-by: Wei Fu <fuweid89@gmail.com>
@@ -315,7 +314,7 @@ type MaybeEtcdResponse struct { | |||
Error string | |||
} | |||
|
|||
var EtcdFutureRevErr = errors.New("future rev") | |||
var ErrEtcdFutureRev = errors.New("future rev") |
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.
Did revive linter raise a warning on this?
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.
Yeah.
The error is here
deterministic.go:318:5: error-naming: error var EtcdFutureRevErr should have name of the form ErrFoo (revive)
var EtcdFutureRevErr = errors.New("future rev")
^
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.
thx for the clarification.
@@ -421,7 +421,7 @@ func (tb triggerBlackhole) Available(config e2e.EtcdProcessClusterConfig, proces | |||
return config.ClusterSize > 1 && process.PeerProxy() != nil | |||
} | |||
|
|||
func blackhole(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error { | |||
func blackhole(ctx context.Context, t *testing.T, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error { |
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.
Did revive linter raise a warning on this?
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.
Yes
failpoints.go:424:30: context-as-argument: context.Context should be the first parameter of a function (revive)
func blackhole(t *testing.T, ctx context.Context, member e2e.EtcdProcess, clus *e2e.EtcdProcessCluster, shouldWaitTillSnapshot bool) error {
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.
thx
- name: context-keys-type | ||
severity: error | ||
disabled: false | ||
# TODO: enable the following rules |
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.
What's the plan for the TODO?
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.
Flip disabled: true
?
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.
I mean why they are disabled now and when to enable them?
It isn't a big deal though.
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.
TODO in tests/revive.toml
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.
@ahrtr The TODO was defined in the tests/revive.toml. I can file issue to track this~
Remove old revive_pass in the bash scripts and migirate the revive.toml into golangci linter_settings.
Part of #16610
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.