Personal/dabradley/lint #182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Any locking operation should have its unlock in a defer statement so that the lock is cleanly unlocked in the event of any issues that occur during the processing that occurs while the lock is held.
This helps make the method more readable since the concerns it works on remain near the same level of abstraction. Pushing these details into helper methods makes the logic of the publish method more clear, while giving a clear name to these operations.
These are unnecessary in golang, so keeping all conditionals in the expected format makes glancing through the logic easier by ensuring each check is consistent.
Instead of having different linters and configuration options in the various places we call golangci-lint, this changes how we run it so that it uses a shared configuration. This prevents the case where one of these checks would pass only to fail for the user when the workflows are run prior to merging.
When testing for the existence or lack of errors, it is beneficial to have the test fail at the first unexpected assertion. If we don't, then it is likely that any further processing will obscure the original failure, or even cause a panic in the test.
Improves failure reporting to have the function configured as a testing helper to make the actual failure more clear.
It's not necessary to change the actual process's actual environment to test these values. The t.Setenv functionality ensures that these values are reset at the end of the test so that other testing is not affected.
Shadowing existing language identifiers is unexpected and creates error-prone expectations of how a given identfier might be used.
Since Go 1.22, the scope of loop variables has changed. It is no longer necessary to copy loop vars to prevent accidental reuse.
The '%w' format automatically works with the unwrapping and wrapping of the existing error to provide more useful debugging information.
Accessing nested protobuf fields directly allows for nil pointer deferences. By forcing the use of getter methods, these chains will not cause these errors and will simply return a nil/zero value for the entire expression if one of the links does not exist.
Using named returns can create very brittle logic where zero values are automatically created and returned and can easily be missed by later maintainers. The existing usage did not leverage the actual named return logic and can be safely removed.
GCI ensures consistent and readable import ordering
For consistent logging and debugging, we should always be using the existing logging framework, rather than printing to standard out.
We are alroady running gofmt, so this just adds further formatting consistency.
k8s-ci-robot
added
the
kind/cleanup
k8s-ci-robot
added
approved
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dabradley, t-mialve 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup