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-action #472

Merged
merged 2 commits into from
Aug 2, 2023
Merged

ci: Use golangci-lint-action #472

merged 2 commits into from
Aug 2, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Aug 1, 2023

Instead of downloading and installing golangci-lint manually,
use the golangci-lint-action.

Besides caching Go-specific files,
this also caches information computed by golangci-lint
so lint checks should run faster.

For this change, I've left the version of golangci-lint
the same as what we were using previously.

@abhinav abhinav added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Aug 1, 2023
Copy link
Contributor Author

abhinav commented Aug 1, 2023

@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch 2 times, most recently from 5fa8ea8 to 483d955 Compare August 1, 2023 18:34
@abhinav abhinav changed the base branch from abhinav/tidy to abhinav/golangci-lint-upgrade August 1, 2023 18:47
@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch from 483d955 to 5a8bac4 Compare August 1, 2023 18:47
@abhinav abhinav force-pushed the abhinav/golangci-lint-upgrade branch from 56e26fe to be9412d Compare August 1, 2023 19:46
@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch from 48c2e13 to 8958d37 Compare August 1, 2023 19:46
@abhinav abhinav force-pushed the abhinav/golangci-lint-upgrade branch from be9412d to cc40264 Compare August 1, 2023 20:42
@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch from 8958d37 to 15825cf Compare August 1, 2023 20:43
@abhinav abhinav mentioned this pull request Aug 1, 2023
@abhinav abhinav force-pushed the abhinav/golangci-lint-upgrade branch from cc40264 to 7079998 Compare August 1, 2023 22:14
@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch from 15825cf to 7bae1fe Compare August 1, 2023 22:14
@abhinav abhinav requested a review from a team August 1, 2023 22:44
Copy link
Contributor Author

abhinav commented Aug 1, 2023

@abhinav started a stack merge that includes this pull request via Graphite.

@abhinav abhinav force-pushed the abhinav/golangci-lint-upgrade branch from 7079998 to f7c39e5 Compare August 1, 2023 23:13
Base automatically changed from abhinav/golangci-lint-upgrade to main August 1, 2023 23:28
Instead of downloading and installing golangci-lint manually,
use the golangci-lint-action.

Besides caching Go-specific files,
this also caches information computed by golangci-lint
so lint checks should run faster.

For this change, I've left the version of golangci-lint
the same as what we were using previously.
Before using golangci-lint-action, lint failures were ignored
as the check was:

    make lint-golang || true

This fixes the issues found by the linter minus revive's
"unused parameter" check which is not something we agree with.
This matches what we've done in pulumi/pulumi.
Copy link
Contributor Author

abhinav commented Aug 1, 2023

Graphite rebased this pull request as part of a merge.

@abhinav abhinav force-pushed the abhinav/golangci-lint-action branch from 7bae1fe to 367607b Compare August 1, 2023 23:28
Copy link
Contributor Author

abhinav commented Aug 1, 2023

Graphite couldn't merge this PR because it failed optional checks and "ignore optional checks" was not selected.

@abhinav
Copy link
Contributor Author

abhinav commented Aug 2, 2023

Flaky test (#481). Retrying

@abhinav abhinav merged commit a5d1dfe into main Aug 2, 2023
4 checks passed
@abhinav abhinav deleted the abhinav/golangci-lint-action branch August 2, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants