Skip to content
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

Modify Runner to use Open3.capture3 #421

Merged
merged 1 commit into from
Mar 18, 2016
Merged

Modify Runner to use Open3.capture3 #421

merged 1 commit into from
Mar 18, 2016

Conversation

seanpdoyle
Copy link
Contributor

Open3.capture3 captures STDOUT and STDERR, allowing the
Runner to manually redirect their output.

This commit modified Runner to only output errors to STDERR
(instead of to both STDOUT and STDERR).

@seanpdoyle
Copy link
Contributor Author

@klaustopher does this PR resolve your noisy output issue?

Does it fail-fast?

@seanpdoyle
Copy link
Contributor Author

@thoughtbot/shell (sorry if this doesn't apply to all of you):

Can someone well-versed in Ruby pipes help me modify this implementation to use Open3.popen3 instead of Open3.capture3?

Essentially, that PR “redirects” output from STDOUT and STDERR to multiple output streams (i.e. a log file and $stdout), with the drawback that it waits for the command to execute before writing.

Ideally, we’d stream from STDOUT and STDERR to the output streams as soon as output is available.

@klaustopher
Copy link

@seanpdoyle yep, works fine ... Only outputs each error message once 👍

[`Open3.capture3`][docs] captures `STDOUT` and `STDERR`, allowing the
`Runner` to manually redirect their output.

This commit modified `Runner` to **only** output errors to `STDERR`
(instead of to both `STDOUT` and `STDERR`).

[docs]: http://ruby-doc.org/stdlib-1.9.3/libdoc/open3/rdoc/Open3.html#method-c-capture3
@seanpdoyle seanpdoyle merged commit 8c6a208 into master Mar 18, 2016
@seanpdoyle seanpdoyle deleted the errors-to-stderr branch March 18, 2016 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants