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

Update linter #2725

Merged
merged 6 commits into from
Mar 20, 2020
Merged

Update linter #2725

merged 6 commits into from
Mar 20, 2020

Conversation

anyasabo
Copy link
Contributor

@anyasabo anyasabo commented Mar 18, 2020

I ran into a bug when troubleshooting the linter failure in #2724 that is resolved in newer golangci-lint versions (a lot of discussion in golangci/golangci-lint#827).

Fixes a bunch of linter warnings:

  • Corrects duplicate imports and ensures receiver names are uniform.
  • I also added an exception for cyclomatic complexity in one of our init functions because I think it's okay in that particular area.

@anyasabo anyasabo added the :ci Things related to Continuous Integration, automation and releases label Mar 18, 2020
Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

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

LGTM

.golangci.yml Outdated
@@ -19,6 +19,7 @@ linters:
- scopelint
- whitespace # added in 1.19
- wsl # added in 1.20
- gomnd # added in 1.23
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it would be nice to maintain the sorted order in this list

.golangci.yml Outdated
- linters:
- stylecheck
text: 'ST1016: methods on the same type should have the same receiver name.*"in".*'
- path: cmd/manager/main\.go
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this list was roughly sorted by path. No big deal though.

Copy link
Contributor Author

@anyasabo anyasabo Mar 19, 2020

Choose a reason for hiding this comment

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

Ah I didn't realize for either of them, fixing

Copy link
Contributor

@charith-elastic charith-elastic left a comment

Choose a reason for hiding this comment

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

LGTM

@anyasabo
Copy link
Contributor Author

jenkins test this please

@anyasabo
Copy link
Contributor Author

I actually changed this to the next to latest version of golangci-lint because it is OOMing jenkins. Already open issue here:
golangci/golangci-lint#994

And example test run failure of previous version of this PR with 1.24.0:
https://devops-ci.elastic.co/job/cloud-on-k8s-pr/3002/consoleFull

@anyasabo
Copy link
Contributor Author

jenkins test this please

@anyasabo
Copy link
Contributor Author

jenkins test this please

@anyasabo anyasabo merged commit 0404e80 into elastic:master Mar 20, 2020
@anyasabo anyasabo deleted the lintbump branch March 20, 2020 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ci Things related to Continuous Integration, automation and releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants