Adding linters to CI #78
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
manuelnaranjo
force-pushed
the
mnaranjo/linters
branch
from
3b9b438 to
ae89571
Compare
|
@rmohr this one is also ready, it will add 2 linters to the CI: buildifier and gazelle, but we could extend to go fmt later as well for instance. It will become faster once I add moving to bazel 7 and use prebuilt protoc |
|
LGTM. Needs a rebase tough, since I merged the CI improvements. |
Adding a CI job that will help us keep the starlark files tidy
now most modules and methods are documented
We have a few cases where we want to warn the user about things, in those cases we want the warning statement to happen
For some reason the linter recomends having a name argument, and also making it explicit which are the required arguments to the methods
When adding toolchain we missed this
When mutating arrays the linter prefers to use extend/append instead of += to the existing array. Also moving from arrays to ctx.actions.attr instead of arrays when passing values into ctx.actions, this reduces the memory preassure during the bazel analysis phase and delays it until the action graph gets executed.
Now we have a gazelle check in CI as well as we have buildifier
manuelnaranjo
force-pushed
the
mnaranjo/linters
branch
from
ae89571 to
b70cdac
Compare
|
@rmohr it's green now |
rmohr
approved these changes
Jul 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing linters in CI, now we run both buildifier and gazelle during CI to make sure we have no pending changes