-
Notifications
You must be signed in to change notification settings - Fork 205
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
Remove dependency on tee
command
#420
Conversation
err = StringIO.new | ||
runner = EmberCli::Runner.new(err: err, out: out) | ||
it "writes to all `err` and `out` streams" do | ||
output_streams = 4.times.map { StringIO.new } |
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.
Use Array.new
with a block instead of .times.map
.
Closes [#417]. Instead of piping output to the [`tee`][tee] command (which was previously swallowing non-zero exit statuses), capture output from `Open3.capture2e` and write to both the specified log file and `STDOUT`. [#417]: #417 [tee]: http://man7.org/linux/man-pages/man1/tee.1.html
This is my favorite solution yet. |
I really like "faking" an IO object that than writes out to multiple other IO objects ... ❤️ the approach! The approach is working for me 👍
The only thing that is happening now, is that it tries to compile all three of our ember apps before failing ... But this is totally fine and I don't think it's worth to putting in any more efforts for fixing this ;)
Thanks @seanpdoyle for all the efforts!!! I cannot thank you enough for getting this sorted out! Should we keep this branch in our Gemfile or can we expect a release soon? |
@klaustopher once CI is green, we'll merge this into
This is unintended behavior. I'd like to resolve this before cutting a release. |
@seanpdoyle That sounds good. Let me know when you have pushed some changes so I can test. Will be available in the next 2 hours. |
end | ||
|
||
def write_to_err(output) | ||
write(out_streams + err_streams, output) |
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.
@thoughtbot/shell does it make sense to write errors to both STDERR
and STDOUT
?
I'm having trouble using git blame
to find the commit that introduced (and justified) this behavior (this PR is only a refactoring of the behavior).
Is it unconventional to only log errors to STDERR
?
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.
Just one, please. It makes the most sense to send errors to stderr.
Closes #417.
Instead of piping output to the
tee
command (which waspreviously swallowing non-zero exit statuses), capture output
from
Open3.capture2e
and write to both the specified log file andSTDOUT
.