-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add missing return after utils.Error() #17364
Conversation
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, eriksjolund 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 |
@eriksjolund TYVM! |
LGTM |
I removed the API change label as it does not impact the API but rather is a bug fix on both accounts. |
@baude In the PR I added another possible missing return. (I hope you noticed that)
I wrote "none". Please advice me if something else should be written as user-facing change. |
I did see it; LGTM. But that is why it takes 2 reviewers to merge stuff. |
6e1d2a6
to
61d7c58
Compare
That's a good thing. (Reducing the chance for mistakes). I added 8 more "missing return" candidates. |
Sounds like we could need a linter for this. Very easy to miss in review otherwise. You have to add |
61d7c58
to
57c92b3
Compare
Yes, it would be great if such missing returns could be caught automatically. I added |
LGTM |
Add missing return after utils.Error(), utils.InternalServerError(), utils.BadRequest(). [NO NEW TESTS NEEDED] Signed-off-by: Erik Sjölund <[email protected]>
57c92b3
to
83a0e97
Compare
Now rebased. |
/cherry-pick v4.4 |
@rhatdan: once the present PR merges, I will cherry-pick it on top of v4.4 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/lgtm |
@rhatdan: new pull request created: #17390 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Erik Sjölund [email protected]
Does this PR introduce a user-facing change?
[NO NEW TESTS NEEDED}