-
Notifications
You must be signed in to change notification settings - Fork 267
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(lint): remove GitHub workflow runtime deprecations, ensure lint is up to date #1242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bumping the outdated deps. I don't understand how this closes #1000 because it looks like none of the lint issues identified in that issue are actually addressed in this PR.
@rootulp 1000 was the trigger to try and look, but couldn't repro those warnings/errors anymore so cleaned up the rest while in there. |
c79d022
go.mod | ||
go.sum | ||
- uses: golangci/golangci-lint-action@v3.7.0 | ||
go-version-file: 'go.mod' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[no change needed] 😮 clever! We can remove a few explicit Go versions from CI jobs in celestia-app with this trick.
Ref: https://github.com/actions/setup-go?tab=readme-ov-file#getting-go-version-from-the-gomod-file
# Required: the version of golangci-lint is required and | ||
# must be specified without patch version: we always use the | ||
# latest patch version. | ||
version: v1.55.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional] we could get rid of this comment b/c it's incorrect and bump to latest golangci-lint https://github.com/golangci/golangci-lint/releases/tag/v1.56.2
but we can def do this in a follow up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! It may be good to reconsider the Closes #1000
in the PR description as that issue seems to be closed already.
Agreed. Updated the PR description. |
Description
Decided to take a look at #1000 and see what was up, seems linting DOES run and is not catching issues that are passed locally, but, while here i noticed some very out of date deprecation warnings on github action runtimes. So:
actions/checkout
tov3
->v4
actions/setup-go
tov4
->v5
git diff
action was stuck on node16 runtime and deprecated, so replicated behavior with paths in workflow trigger, even though it does require 2 entries (1 for push, 1 for pull_request) seemed cleaner than an unmaintained github action workflowsetup-go
to usego.mod
for sourcing the go version.If welcome, i intend to update warnings across all workflows next.
PR checklist
.changelog
(we useunclog to manage our changelog)
docs/
orspec/
) and code comments