-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: debug golangci-lint for cosmovisor #18304
Conversation
WalkthroughThe changes primarily revolve around modifying test and linting commands for various packages and modules in the project. The Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .github/workflows/test.yml (29 hunks)
- scripts/go-lint-all.bash (2 hunks)
- tools/cosmovisor/args.go (1 hunks)
Files skipped from review due to trivial changes (2)
- scripts/go-lint-all.bash
- tools/cosmovisor/args.go
Additional comments: 1
.github/workflows/test.yml (1)
- 1185-1187: The changes in these hunks are removing the
rocksdb_build
tag from thego test
command. This is likely due to the fact that therocksdb_build
tag was causing issues with the linter. Ensure that removing this tag does not affect the functionality of the tests or the packages they are testing.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tools/cosmovisor/process.go (1 hunks)
Files skipped from review due to trivial changes (1)
- tools/cosmovisor/process.go
else | ||
golangci-lint run ./... -c "${REPO_ROOT}/.golangci.yml" "$@" --build-tags=rocksdb,e2e,ledger,test_ledger_mock | ||
fi | ||
if [[ (-z "${NIX:-}" && $f != store) || $f == "tools/"* ]]; then |
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.
unclear how this effects cosmovisor? can you explain
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.
Yes, $f is the directory name/path, so if it contains tools/ it applies to confix and cosmovisor
cosmovisor is under tools/, we actually have the same issue for all tools (confix).
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.
is there a flag in the list that is needed for the tools? that was the confusing part,
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.
Yes, using rocksdb tag for building tooling breaks them.
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.
ah okay, thank you
Description
PRs with cosmovisor changes are blocked due to failing linting.
This probably have been introduced with the extended linting script and the combination of building tags.
This PR addresses the issue and removes the
rocksdb_build
tag from ci, as it does not exist.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
make lint
andmake test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking changeSummary by CodeRabbit
Tests:
rocksdb_build
tag for improved test execution.Style:
lint_module()
function for better code readability.Chores:
args.go
for better code understanding and maintenance.