ktlint_test windows support
#785
Conversation
This helper is an exact copy of the file from https://github.com/aspect-build/bazel-lib/blob/df04df7cd2b9a481787de1cb43a6fe05f67c9511/lib/windows_utils.bzl Which in turn I believe borrows heavily from rules_nodejs and bazel internal helpers.
Unfortunately, the existing shell script is not launchable by the default windows CreateProcess command. See https://bazel.build/rules/windows_tips#actions for recommendations on how to handle this. I opted to deal with it in the same way as rules_nodejs does.
Taking https://bazel.build/rules/depsets and https://bazel.build/rules/performance into account, it is better not to call `to_list()` on a depset.
hanneskaeufler
requested review from
cgruber,
jin,
restingbull and
timpeut
as code owners
|
@hanneskaeufler can you include this in the windows CI PR so that we can validate if this works? |
This also ensures both the build as well as the test are run for windows and linux. Previously, the tests, and thus the lint target, was not run. Hence it was not apparent that the target failed, because the sample app is formatted incorrectly.
This makes the tests in this example pass for both windows and linux
So no need to specify options for all of them.
As mentioned in the rule docs.
Yeah sure @Bencodes, I was contemplating how to do this since actually the linux example stage did not test the linter at all as well, and as mentioned in the description the file was not formatted to pass the linter. But I assume now that this was not intentional and thus enabled the tests in the example stage for both windows and linux by reformatting the file. Enabled now in this PR and passing 🍒 |
|
|
||
| data class Foo(val name: String, val age: Int) | ||
|
|
||
| class Query { |
There was a problem hiding this comment.
@hanneskaeufler Did you mean to pull in these changes?
There was a problem hiding this comment.
Yes, file needed to be reformatted to pass the linter
|
Could the same code be used to enable Windows support for |
|
Probably something along those lines should work, I will be looking into that over the coming week(s). I just started with the linter because that was easiest to get familiar with the codebase 😂 |
3 participants
This enables windows linting support with the
ktlint_testrule.Previously this rule would fail such as:
Now it runs successfully. However, the actual files being linted there fail the linter due to wrong indentation.
I'll probably also submit a PR that fixes indentation so we can runtest //...on CI for the trivial examples.Included in this PR now is also the CI changes to proof that it works.