-
Notifications
You must be signed in to change notification settings - Fork 5
Update run_swiftlint
to allow for custom .swiftlint config files
#189
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
Conversation
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.
Approving to unblock, with an open question
bin/run_swiftlint
Outdated
echo "Usage: $0 [--strict | --lenient] [--config CONFIG_FILE]" | ||
exit 1 | ||
fi | ||
CONFIG_FILE="$2" |
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.
Should we allow multiple occurrences of the --config
flag like the SwiftLint
binary does?
I.e. make CONFIG_FILE=()
be initialized as an array and do CONFIG_FILE+=
here?
Otherwise invoking run_SwiftLint
with multiple --config A --config B
, while valid with swiftlint
, will make run_SwiftLint
only consider the last one and ignore the other ones, which might be a confusing discrepancy with the official swiftlint
call…
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.
I wasn't considering SwiftLint could deal with multiple --config
-- nice one, added support for it on 0b5fdc8
bin/run_swiftlint
Outdated
# If no config files specified, use default | ||
if [[ ${#CONFIG_FILES[@]} -eq 0 ]]; then | ||
CONFIG_FILES=(".swiftlint.yml") | ||
SWIFTLINT_ARGUMENTS+=(--config ".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.
I wonder if that SWIFTLINT_ARGUMENTS+=(--config ".swiftlint.yml")
is strictly necessary, since that's already handled by swiftlint
itself that if no --config …
is passed it uses .swiftlint.yml
by default… so might as well not pass that --config
argument in those cases?
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.
Hmm not necessary indeed 🤔 updated on aaa2860
- Add `--config` argument to `run_swiftlint` script to allow specifying custom `.swiftlint.yml` config files and allowing for multiple `--config` arguments [#189]
- Add `--config` argument to `run_swiftlint` script to allow specifying custom `.swiftlint.yml` config files and allowing for multiple `--config` arguments [#189]
- Add `--config` argument to `run_swiftlint` script to allow specifying custom `.swiftlint.yml` config files and allowing for multiple `--config` arguments [#189]
Related to https://github.com/Automattic/buildkite-ci/pull/707, this PR updates the
run_swiftlint
script to allow the--config
parameter specifying a SwiftLint config file.CHANGELOG.md
if necessary.