Skip to content

doc: improve text for breakOnSigint - #35692

Merged
Trott merged 1 commit into
nodejs:masterfrom
Trott:breakOnSigInt
Oct 24, 2020
Merged

doc: improve text for breakOnSigint#35692
Trott merged 1 commit into
nodejs:masterfrom
Trott:breakOnSigInt

Conversation

@Trott

@TrottTrott commented Oct 17, 2020

Copy link
Copy Markdown
Member
  • Make the 7 instances of breakOnSigint text blocks consistent.
  • Use present tense.
  • Use kbd element for keystrokes.
  • Minor style edits.
Checklist

@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. vm Issues and PRs related to the vm subsystem. labels Oct 17, 2020
Trott
Trott commented Oct 17, 2020
View reviewed changes
Comment threaddoc/api/vm.md Outdated

@TrottTrottOct 17, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The word interrupted here is the one thing I didn't modify for consistency. The other six instances say terminated. I wasn't completely sure the choice wasn't intentional or which word was better than the other. @nodejs/vm @nodejs/documentation

@addaleaxaddaleaxOct 18, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think interrupt is a good word, especially given that that’s what SIGINT stands for :)

@aduh95aduh95Oct 24, 2020

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.

Should it be interrupted on the other six instances then? I think consistency would be good, and @addaleax makes a good point.

@TrottTrottOct 24, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Should it be interrupted on the other six instances then?

I'll land this as-is and open a follow-on PR about that.

@Trott

Trott commented Oct 18, 2020

Copy link
Copy Markdown
MemberAuthor

Before:

image

After:

image

@Trott
doc: improve text for breakOnSigint
6bf2139
* Make the 7 instances of breakOnSigint text blocks consistent.
* Use present tense.
* Use kbd element for keystrokes.
* Minor style edits.
PR-URL: nodejs#35692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott
Trott merged commit 6bf2139 into nodejs:masterOct 24, 2020
@Trott

Trott commented Oct 24, 2020

Copy link
Copy Markdown
MemberAuthor

Landed in 6bf2139

@Trott
Trott deleted the breakOnSigInt branch October 24, 2020 12:16
targos pushed a commit that referenced this pull request Nov 3, 2020
@Trott@targos
doc: improve text for breakOnSigint
890b03e
* Make the 7 instances of breakOnSigint text blocks consistent.
* Use present tense.
* Use kbd element for keystrokes.
* Minor style edits.
PR-URL: #35692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targostargos mentioned this pull request Nov 3, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Dec 8, 2020
@Trott@BethGriggs
doc: improve text for breakOnSigint
c15e8a5
* Make the 7 instances of breakOnSigint text blocks consistent.
* Use present tense.
* Use kbd element for keystrokes.
* Minor style edits.
PR-URL: #35692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit that referenced this pull request Dec 10, 2020
@Trott@BethGriggs
doc: improve text for breakOnSigint
f2e7560
* Make the 7 instances of breakOnSigint text blocks consistent.
* Use present tense.
* Use kbd element for keystrokes.
* Minor style edits.
PR-URL: #35692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@BethGriggsBethGriggs mentioned this pull request Dec 10, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Dec 15, 2020
@Trott@BethGriggs
doc: improve text for breakOnSigint
47ba122
* Make the 7 instances of breakOnSigint text blocks consistent.
* Use present tense.
* Use kbd element for keystrokes.
* Minor style edits.
PR-URL: #35692
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleaxaddaleaxaddaleax left review comments
@aduh95aduh95aduh95 left review comments
@lpincalpincalpinca approved these changes
+2 more reviewers
@aymen94aymen94aymen94 approved these changes
@randomhashrandomhashrandomhash approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docIssues and PRs related to the documentations.vmIssues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott@addaleax@lpinca@aymen94@aduh95@randomhash@nodejs-github-bot