Skip to content

Conversation

The static check doesn't know that cobra will exit prematurely. And
thinks that leaderCli can be nil later on:

../etcdctl/ctlv3/command/move_leader_command.go:78:14: SA5011: possible nil pointer dereference (staticcheck)
        resp, err = leaderCli.MoveLeader(ctx, target)

Add a return (although unreachable), so SA5011 doesn't flag this issue.

Signed-off-by: Ivan Valdes <[email protected]>
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivanvc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ivanvc ivanvc force-pushed the dependency-update-20251103 branch from 4c0c09b to bbd8431 Compare November 8, 2025 00:10
@codecov
Copy link

codecov bot commented Nov 8, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 69.18%. Comparing base (ae91d30) to head (bbd8431).

Files with missing lines Patch % Lines
etcdctl/ctlv3/command/move_leader_command.go 0.00% 1 Missing ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
etcdctl/ctlv3/command/move_leader_command.go 0.00% <0.00%> (ø)

... and 25 files with indirect coverage changes

@@           Coverage Diff           @@
##             main   #20907   +/-   ##
=======================================
  Coverage   69.17%   69.18%           
=======================================
  Files         422      422           
  Lines       34839    34840    +1     
=======================================
+ Hits        24100    24104    +4     
+ Misses       9341     9335    -6     
- Partials     1398     1401    +3     

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae91d30...bbd8431. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ivanvc
Copy link
Member Author

ivanvc commented Nov 8, 2025

/retest

}
if leaderCli == nil {
cobrautl.ExitWithError(cobrautl.ExitBadArgs, fmt.Errorf("no leader endpoint given at %v", eps))
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add this? Did the linter complain this?
It will never be executed. cobrautl.ExitWithError exits the process.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I documented this in the ea665b5 commit message.

The static check doesn't know that cobra will exit prematurely. And thinks that leaderCli can be nil later on:

../etcdctl/ctlv3/command/move_leader_command.go:78:14: SA5011: possible nil pointer dereference (staticcheck)
resp, err = leaderCli.MoveLeader(ctx, target)

Add a return (although unreachable), so SA5011 doesn't flag this issue.

The other option would be to add a comment to ignore the warning.

Reference: https://staticcheck.dev/docs/checks/#SA5011

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, in that case, pls add a comment above to clarify why only adding return here (note that there are lots of cobrautl.ExitWithError usage). Not everyone read commit message.

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

Development

Successfully merging this pull request may close these issues.

3 participants