-
Notifications
You must be signed in to change notification settings - Fork 51
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
iOS: Add swiftlint as ios test #364
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## bazel-6 #364 +/- ##
========================================
Coverage 91.70% 91.70%
========================================
Files 332 332
Lines 26714 26714
Branches 1935 1935
========================================
Hits 24499 24499
Misses 2202 2202
Partials 13 13 ☔ View full report in Codecov by Sentry. |
ios/BUILD.bazel
Outdated
"//plugins/types-provider/ios:PlayerUITypesProviderPluginTests" | ||
"//plugins/types-provider/ios:PlayerUITypesProviderPluginTests", | ||
|
||
"@SwiftLint//:swiftlint" |
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.
do we need this? did it end up doing something useful?
tools/ios/util.bzl
Outdated
|
||
native.sh_test( | ||
name = name + "SwiftLint", | ||
srcs = [":"+ name + "_Lint"], | ||
visibility = ["//visibility:public"], | ||
) | ||
|
||
native.genrule( | ||
name = name + "_Lint", | ||
tools = [ | ||
"@SwiftLint//:swiftlint" | ||
], | ||
srcs = native.glob(["Sources/**/*.swift"]) + ["//:.swiftlint.yml"], | ||
outs = ["output.sh"], | ||
executable = True, | ||
visibility = ["//visibility:public"], | ||
cmd=""" | ||
echo `$(location @SwiftLint//:swiftlint) --config $(location //:.swiftlint.yml) $(SRCS) || true` > lint_results.txt | ||
LINT=$$(cat lint_results.txt) | ||
|
||
echo '#!/bin/bash' > $(location output.sh) | ||
echo "echo '$$LINT'" > $(location output.sh) | ||
|
||
LINESWITHERROR=$$(echo grep error lint_results.txt || true) | ||
echo "exit $$(($$LINESWITHERROR) | wc -l)" >> $(location output.sh) | ||
""" | ||
) | ||
|
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.
can you move the lint definitions to the bottom of the macro?
The doc block has to be at the top of the function to meet the docstring format
tools/ios/util.bzl
Outdated
tools = [ | ||
"@SwiftLint//:swiftlint" | ||
], | ||
srcs = native.glob(["Sources/**/*.swift"]) + ["//:.swiftlint.yml"], |
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.
if you move this further down the macro, you can use the same target to refer to sources that we use for the swift_library
and for bundling the zip
srcs = native.glob(["Sources/**/*.swift"]) + ["//:.swiftlint.yml"], | |
srcs = [":" + name + "_Sources"] + ["//:.swiftlint.yml"], |
BUILD.bazel
Outdated
@@ -248,3 +248,6 @@ gazelle( | |||
name = "update_build_files", | |||
gazelle = ":gazelle_bin", | |||
) | |||
|
|||
#SwiftLint | |||
exports_files([".swiftlint.yml", "output.sh"]) |
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.
exports_files([".swiftlint.yml", "output.sh"]) | |
exports_files([".swiftlint.yml"]) |
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.
🚀
Change Type (required)
Indicate the type of change your pull request is:
patch
minor
major
Does your PR have any documentation updates?