-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Added option to set path to lint using swiftlint action #12197
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.
Looks like a nice improvement. Just added one small suggestion.
FastlaneCore::ConfigItem.new(key: :path, | ||
description: "Specify path to lint", | ||
is_string: true, | ||
optional: true), |
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 we verify the path exists using ConfigItem
's verify_block
argument?
verify_block: proc do |value|
UI.user_error!("Couldn't find path '#{File.expand_path(value)}'") unless File.exist?(value)
end),
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.
Done.
Updated tests
AppVeyor failed on new unit test, but actually I don't know how to solve that. I don't have too much knowledge on Ruby, probably I did my test wrongly. @getaaron could take a look please? Here is the error:
|
path: '#{path}' | ||
) | ||
end").runner.execute(:test) | ||
end.to raise_error("Couldn't find path '#{path}'") |
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.
The issue with the failing Windows test is it is putting a C:
in front of the error so its not matching that "exact" string. Try... end.to raise_error(/Couldn't find path/)
end | ||
|
||
it "adds invalid path option" do | ||
path = "/path/to/lint" |
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.
Or maybe try changing this to a relative path path = "./path/to/lint"
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.
Thanks for the addition! 😊
Hey @bguidolim 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
Great work @bguidolim! Thanks for your help on this @joshdholtz |
Congratulations! 🎉 This was released as part of fastlane 2.90.0 🚀 |
Checklist
bundle exec rspec
from the root directory to see all new and existing tests passbundle exec rubocop -a
to ensure the code style is validMotivation and Context
Currently we can use
swiftlint
action to run in the current folder, but if you want to runswiftlint
only in a specific subfolder or other path outside of the current path is not possible.This PR adds the option
path
toswiftlint
action, so now it's possible to specify the path you want to run following the default behaviour ofswiflint
treatment for this specific folder.I didn't find any issue related.