-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Avoid mutating $stdout and $stderr #2486
Avoid mutating $stdout and $stderr #2486
Conversation
9488cd3
to
89fc1e3
Compare
May want to rebase against master as Greg just fixed an issue where we were using puts instead of stdout |
89fc1e3
to
6de31e4
Compare
2d4d339
to
0f278af
Compare
I have tried with different apps locally and on Heroku and everything works fine. Maybe someone has a clear case that should be tested with this change? Ideas? |
52a563a
to
bdfb27a
Compare
Uses `flush` after every write if the stdio it is not synchronized. Closes: puma#1948
bdfb27a
to
f519468
Compare
Also, I have mixed feelings now because I introduced some duplication, do you think it's a good idea to encapsulate the new writing logic for the code outside |
Re: correctness, anything from #1906 could be extracted as a test |
You can leave the refactor/DRYness for another PR if you want, don't let it block this feature. |
Ideas for test re: comments in that thread:
|
I've been checking the current integration tests and it seems that puma/test/test_integration_single.rb Line 126 in 441c474
|
Took another look, I think this is good as written |
Uses `flush` after every write if the stdio it is not synchronized. Closes: puma#1948 Co-authored-by: Nate Berkopec <nate.berkopec@gmail.com>
Description
Closes #1948
Uses
flush
after writing to stdout and stderr so we don't need to usesync=true
Your checklist for this pull request
[changelog skip]
or[ci skip]
to the pull request title.[ci skip]
to the title of the PR.#issue
" to the PR description or my commit messages.