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

ci: Use golangci-lint for linting #7440

Merged
merged 1 commit into from
Mar 24, 2020
Merged

ci: Use golangci-lint for linting #7440

merged 1 commit into from
Mar 24, 2020

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Mar 12, 2020

Using golangci-lint has a number of advantages:

  • adding new linters becomes much easier, its a couple lines of yaml config
    instead of more bash scripting

  • it enables whitelisting of issues using inline comments or regex

  • when running multiple linters less work is done. The parsed source can be reused
    by multiple linters

  • linters are run in parallel to reduce CI runtime.

This PR also changes the workflow so that more work can be done in parallel. This should reduce the overall workflow runtime by a minute or two.

@dnephin dnephin force-pushed the dnephin/golangci-lint branch 2 times, most recently from e12df0f to e028736 Compare March 13, 2020 17:10
Using golangci-lint has a number of advantages:

- adding new linters becomes much easier, its a couple lines of yaml config
  instead of more bash scripting

- it enables whitelisting of issues using inline comments or regex

- when running multiple linters less work is done. The parsed source can be reused
  by multiple linters

- linters are run in parallel to reduce CI runtime.
Copy link
Contributor

@alvin-huang alvin-huang left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @dnephin! Everything LGTM!

One suggestion to make the local development easier is to either:

  • update the format and vet targets with the relevant golangci-lint commands, ie: golangci-lint run --disable-all -E gofmt

  • or remove them and add one lint target that runs everything

(with a command to go get the tool first in either case)

This can be a followup PR so this is good to go in as is. Great work!

@dnephin dnephin merged commit c765780 into master Mar 24, 2020
@dnephin dnephin deleted the dnephin/golangci-lint branch March 24, 2020 20:01
@dnephin dnephin mentioned this pull request Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants