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

Remove Python 2 use of std{out,err} over std{out,err}.buffer #7968

Merged
merged 1 commit into from Jun 28, 2019

Conversation

Eric-Arellano
Copy link
Contributor

No description provided.

@stuhood
Copy link
Sponsor Member

stuhood commented Jun 28, 2019

This one feels likely to cause cherry-pick conflicts to 1.17.x. Let's hold off a few more days, since @blorente might need one more round to stabilize pantsd there before it is finalized.

@Eric-Arellano
Copy link
Contributor Author

This one feels likely to cause cherry-pick conflicts to 1.17.x.

I don't think it's very likely to cause an actual merge conflict, since the logic around stdout.buffer has been stable for the past few months. The only merge conflict I could imagine is if we were to change any of this code around stdout.buffer, which I don't anticipate happening.

Fine waiting need be, but don't think it's necessary here.

Copy link
Contributor

@blorente blorente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading through the changes, there are a couple of files that are likely to change in the next round of pantsd fixes, (exception_sink.py, exiter.py, pants_daemon.py and remote_pants_runner.py).

However, I find it hard to imagine modifying these specific lines in the pantsd fixes, so I don't think we should halt the progress of upstream for that.

@Eric-Arellano
Copy link
Contributor Author

Only failure is flake from #7856.

@Eric-Arellano Eric-Arellano merged commit 847dbe4 into pantsbuild:master Jun 28, 2019
@Eric-Arellano Eric-Arellano deleted the py3-stdout-buffer branch June 28, 2019 16:37
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.

None yet

3 participants