-
Notifications
You must be signed in to change notification settings - Fork 984
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
emit xcodebuild & other spawned CLI commands #479
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.
I am requesting changes to avoid a premature merge:
- test failures need to be resolved
- It looks like this PR includes the changes from Use cross-spawn & shelljs instead of child-process #478. I would favor that we make all of these changes in Use cross-spawn & shelljs instead of child-process #478 and close this one.
@brodybits I wanted to keep this change in a separate PR so it will be listed as a discrete change in possible changelogs and to better track the individual change. I would rebase this PR when #478 is merged, to get a cleaner merge. |
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 am looking forward to the rebase now that PR Use cross-spawn & shelljs instead of child-process #478 is merged.
- I would favor using the
printCommand: true
option on all "superspawn" commands.
Codecov Report
@@ Coverage Diff @@
## master #479 +/- ##
==========================================
- Coverage 75.71% 75.66% -0.05%
==========================================
Files 11 11
Lines 1795 1796 +1
==========================================
Hits 1359 1359
- Misses 436 437 +1
Continue to review full report at Codecov.
|
This should be all good now :) |
Closes #477
@brodybits I pushed another update to put |
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.
👍
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 again @oliversalzburg
I just updated the title to better reflect the actual changes, will merge in a few minutes or so. |
Merged with title updated again for the sake of clarity. Thanks again @oliversalzburg! |
Platforms affected
iOS
What does this PR do?
It logs the CLI for
xcodebuild
. Please note that this PR expects #477 to be merged also.What testing has been done on this change?
None