-
Notifications
You must be signed in to change notification settings - Fork 266
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
[ToolRunner] Enhanced 'shell' execution option #643
[ToolRunner] Enhanced 'shell' execution option #643
Conversation
Could you add some tests for this change please? |
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.
LGTM but please take a look at these comments.
@damccorm Thank you very much for the review! I've applied changes in accordance with your review points so could you please take a look? |
Failing windows test is unrelated, I'm going to merge |
Enhancement for #637
Required to fix issue #12848.
Required for PR #13099
Applied enhancement for
shell
option to wrap a tool path and arguments with double quotes to ensure that command is handled correctly if it is executed inside shell. The path and arguments are wrapped only ifshell
property ofIExecOptions
object set to true.Added tests.