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.

Sign up for GitHub

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

Jump to bottom

[READY] More fixes for printing with custom sys.stdout #381

Merged
merged 1 commit into from
Feb 21, 2016
Merged

[READY] More fixes for printing with custom sys.stdout #381

merged 1 commit into from
Feb 21, 2016

Conversation

Valloric
Copy link
Member

The fix from #379 fixed py2, but broke py3. This fixes both and adds a test.

Review on Reviewable

@puremourning
Copy link
Member

LGTM assuming the tests pass

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@Valloric Valloric mentioned this pull request Feb 20, 2016
Merged
@micbou
Copy link
Collaborator

micbou commented Feb 20, 2016

:lgtm: with minor comment.

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

ycmd/tests/utils_test.py, line 305 [r1] (raw file):
Extra blank line.

Comments from the review on Reviewable.io

@Valloric
Copy link
Member Author

@homu r=puremourning

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion.

ycmd/tests/utils_test.py, line 305 [r1] (raw file):
Done.

Comments from the review on Reviewable.io

@homu
Copy link
Contributor

homu commented Feb 20, 2016

📌 Commit 0f3a3c1 has been approved by puremourning

@homu
Copy link
Contributor

homu commented Feb 20, 2016

⌛ Testing commit 0f3a3c1 with merge b128d9e...

homu added a commit that referenced this pull request Feb 20, 2016
@homu
Auto merge of #381 - Valloric:stdout, r=puremourning
b128d9e 
[READY] More fixes for printing with custom sys.stdout

The fix from #379 fixed py2, but broke py3. This fixes both and adds a test.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/381)
<!-- Reviewable:end -->
@homu
Copy link
Contributor

homu commented Feb 20, 2016

☔ The latest upstream changes (presumably #380) made this pull request unmergeable. Please resolve the merge conflicts.

@micbou micbou mentioned this pull request Feb 20, 2016
Merged
@Valloric
Copy link
Member Author

@homu retry

@micbou
Copy link
Collaborator

micbou commented Feb 20, 2016

New commit.

@homu r+

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.

Comments from the review on Reviewable.io

@homu
Copy link
Contributor

homu commented Feb 20, 2016

📌 Commit 698441b has been approved by micbou

@homu
Copy link
Contributor

homu commented Feb 20, 2016

⌛ Testing commit 698441b with merge 67c418d...

homu added a commit that referenced this pull request Feb 20, 2016
@homu
Auto merge of #381 - Valloric:stdout, r=micbou
67c418d 
[READY] More fixes for printing with custom sys.stdout

The fix from #379 fixed py2, but broke py3. This fixes both and adds a test.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/381)
<!-- Reviewable:end -->
@micbou
Copy link
Collaborator

micbou commented Feb 20, 2016

@homu r-

Sorry to stop the merging but I have two minor comments.

Reviewed 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

ycmd/tests/utils_test.py, line 313 [r3] (raw file):
Missing blank line.

ycmd/tests/utils_test.py, line 320 [r3] (raw file):
End of line missing.

Comments from the review on Reviewable.io

@micbou
Copy link
Collaborator

micbou commented Feb 20, 2016

And I have no idea if it works.

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Comments from the review on Reviewable.io

@Valloric
More fixes for printing with custom sys.stdout
56046fd 
The fix from #379 fixed py2, but broke py3. This fixes both and adds a
test.
@Valloric
Copy link
Member Author

Should be good now. I'll let you kick off homu. :)

Review status: 2 of 3 files reviewed at latest revision, 2 unresolved discussions.

ycmd/tests/utils_test.py, line 313 [r3] (raw file):
Done.

ycmd/tests/utils_test.py, line 320 [r3] (raw file):
Done.

Comments from the review on Reviewable.io

@micbou
Copy link
Collaborator

micbou commented Feb 20, 2016

@homu r+

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from the review on Reviewable.io

@homu
Copy link
Contributor

homu commented Feb 20, 2016

📌 Commit 56046fd has been approved by micbou

@homu
Copy link
Contributor

homu commented Feb 20, 2016

⌛ Testing commit 56046fd with merge 528dbad...

homu added a commit that referenced this pull request Feb 20, 2016
@homu
Auto merge of #381 - Valloric:stdout, r=micbou
528dbad 
[READY] More fixes for printing with custom sys.stdout

The fix from #379 fixed py2, but broke py3. This fixes both and adds a test.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/381)
<!-- Reviewable:end -->
@homu
Copy link
Contributor

homu commented Feb 21, 2016

☀️ Test successful - status

@homu homu merged commit 56046fd into master Feb 21, 2016
@homu homu mentioned this pull request Feb 21, 2016
Merged
@Valloric Valloric deleted the stdout branch February 21, 2016 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Valloric @puremourning @micbou @homu