Skip to content

tty: handle setRawMode errors#22886

Closed
seishun wants to merge 1 commit into
nodejs:masterfrom
seishun:setRawMode-error
Closed

tty: handle setRawMode errors#22886
seishun wants to merge 1 commit into
nodejs:masterfrom
seishun:setRawMode-error

Conversation

@seishun

@seishun seishun commented Sep 16, 2018

Copy link
Copy Markdown
Contributor

Refs: #21773

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot

nodejs-github-bot commented Sep 16, 2018

Copy link
Copy Markdown
Collaborator

@seishun build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/898/pipeline

@nodejs-github-bot nodejs-github-bot added the tty Issues and PRs related to the tty subsystem. label Sep 16, 2018
@Trott

Trott commented Sep 17, 2018

Copy link
Copy Markdown
Member

Is it possible (and not too onerous) to add a reliable test that will cover the error condition?

@seishun

seishun commented Sep 17, 2018

Copy link
Copy Markdown
Contributor Author

I don't think it's possible to trigger an error intentionally.

@seishun

seishun commented Sep 17, 2018

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/17242/

Fishrock123
Fishrock123 approved these changes Sep 17, 2018
View reviewed changes

@Fishrock123 Fishrock123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is that major? I'm not really sure. I guess in theory errors should be so uncommon that it doesn't matter?

@seishun

seishun commented Sep 17, 2018

Copy link
Copy Markdown
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/17252/

@danbev

danbev commented Sep 19, 2018

Copy link
Copy Markdown
Contributor

Re-run of CI: https://ci.nodejs.org/job/node-test-commit-linux/21689/ was successful.

@danbev

danbev commented Sep 19, 2018

Copy link
Copy Markdown
Contributor

Landed in a82fc30.

@danbev danbev closed this Sep 19, 2018
danbev pushed a commit that referenced this pull request Sep 19, 2018
@seishun @danbev
tty: handle setRawMode errors
a82fc30 
PR-URL: #22886
Refs: #21773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Sep 20, 2018
@seishun @targos
tty: handle setRawMode errors
36028ca 
PR-URL: #22886
Refs: #21773
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos targos mentioned this pull request Oct 7, 2018
Merged
This was referenced Oct 10, 2018
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@seishun seishun deleted the setRawMode-error branch May 2, 2020 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

+1 more reviewer

@Fishrock123 Fishrock123 Fishrock123 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tty Issues and PRs related to the tty subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@seishun @nodejs-github-bot @Trott @danbev @addaleax @Fishrock123 @cjihrig