Conversation
|
It'd also be awesome if you could cut a release after this PR 🙏 |
|
@aignas friendly ping here 🙏 |
aignas
left a comment
aignas
left a comment
There was a problem hiding this comment.
Thank you for all of the contributions.
| ) | ||
|
|
||
| toolchain = ctx.toolchains[TOOLCHAIN_TYPE] | ||
| executable = ctx.actions.declare_file("{}{}".format( |
There was a problem hiding this comment.
Ideally, I would like windows addition to be a separate PR, but it is OK this time.
| alias( | ||
| name = "shellcheck", | ||
| actual = "shellcheck.exe", | ||
| ) | ||
|
|
||
| alias( | ||
| name = "{name}", | ||
| actual = "shellcheck.exe", | ||
| ) | ||
|
|
||
| shellcheck_toolchain( | ||
| name = "toolchain", | ||
| shellcheck = ":shellcheck", | ||
| ) |
There was a problem hiding this comment.
nit it would be nice to have a single macro here. That we if we update the macro implementation (e.g. change the alias), then we don't need to refetch everything.
There was a problem hiding this comment.
I'm not sure what you mean by "single macro". Can you expand?
| alias( | ||
| name = "{name}", | ||
| actual = "shellcheck.exe", | ||
| ) |
There was a problem hiding this comment.
Could you please explain why we have an extra alias here instead of just {name} and no shellcheck?
There was a problem hiding this comment.
I want the consistency of being able to refer to a :shellcheck target but for convenience it's nice to be able to just refer to @repo_name without needing to specify target so I have an alias for both.
|
|
||
| on: # yamllint disable rule:truthy | ||
| push: | ||
| branches: |
There was a problem hiding this comment.
In our CI we are not testing Windows at the moment, but we are adding Windows support in here, shall we add it to our build matrix here?
There was a problem hiding this comment.
This improves windows support but I didn't add full windows support. That would require a much larger dump of batch to account for runfiles manifests in tests. I can add that to this PR if you want but already felt like it was ballooning quickly. Just let me know
2 participants
This allows for users to provide custom shellcheck binaries to power the shellcheck rules.
Additionally, this adds support for
.shellcheckrcfiles via--@rules_shellcheck//shellcheck:rc=<label_of_file>closes #24