Skip to content
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

Adjust golangci-lint linters #1700

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from

Conversation

tpantelis
Copy link
Contributor

Added new linters (some commented out if not needed) and removed deprecated linters. Also addressed violations reported by new linters (in separate commits).

Related to submariner-io/enhancements#231

Added new linters (some commented out if not needed) and removed
deprecated linters. Also addressed violations reported by new linters
(in separate commits).

Related to submariner-io/enhancements#231

Signed-off-by: Tom Pantelis <[email protected]>
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr1700/tpantelis/adj_linters
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

Copy link
Member

@skitt skitt left a comment

Choose a reason for hiding this comment

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

I still would like to see “linter” commit descriptions that focus on the improvement to the project rather than the linter that flags the issues — these changes are worth doing because they are an improvement, not to fix a linter violation.

In this case the commit series would look like this:

Remove duplicate words ("on")

This was flagged by dupword.
Use resource.toJSON instead of json.MarshalIndent

This avoids having to check errors everywhere (flagged by errchkjson).
Use integer ranges in for loops

This was flagged by intrange.
Ignore maintidx on purposefully "unmaintainable" functions
Stop using named returns

This is now enforced by nonamedreturns.
Remove unnecessary formatting functions

* string concatenation instead of fmt.Sprintf
* errors.New instead of fmt.Errorf

This is now enforced by perfsprint.

Hopefully that helps illustrate why I think this approach is better. When reviewing the code history later, it’s more immediately apparent what the change is about.

This was flagged by the `dupword` linter.

Signed-off-by: Tom Pantelis <[email protected]>
This avoids having to check errors everywhere (flagged by the
'errchkjson' linter).

Signed-off-by: Tom Pantelis <[email protected]>
This was flagged by the `intrange` linter.

Signed-off-by: Tom Pantelis <[email protected]>
This is now enforced by the 'nonamedreturns' linter.

Signed-off-by: Tom Pantelis <[email protected]>
* string concatenation instead of fmt.Sprintf
* errors.New instead of fmt.Errorf

This is now enforced by the 'perfsprint' linter.

Signed-off-by: Tom Pantelis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants