Skip to content

Conversation

PoojaDurgad
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Oct 10, 2020
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
that started the Node.js process. Symbolic links if any, will be resolved
that started the Node.js process. Symbolic links, if any, will be resolved

Copy link
Member

Choose a reason for hiding this comment

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

In addition to the comma, I'd prefer that we keep things in present tense if there's a choice:

Suggested change
that started the Node.js process. Symbolic links if any, will be resolved
that started the Node.js process. Symbolic links, if any, are resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@addaleax and @Trott - thanks for the review, I have followed your suggestion. Please have a look.

Comment on lines 1189 to 1190
Copy link
Member

@Trott Trott Oct 11, 2020

Choose a reason for hiding this comment

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

Maybe also leave off the last three words?

Suggested change
that started the Node.js process. Symbolic links if any, will be resolved
in the pathname.
that started the Node.js process. Symbolic links, if any, are resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Trott -Thanks. I have followed your suggestion. Please have a look.

PR-URL: nodejs#35590
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott Trott merged commit 8c177c4 into nodejs:master Oct 13, 2020
@Trott
Copy link
Member

Trott commented Oct 13, 2020

Landed in 8c177c4

MylesBorins pushed a commit that referenced this pull request Oct 14, 2020
PR-URL: #35590
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 14, 2020
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
PR-URL: #35590
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2020
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
PR-URL: #35590
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
PR-URL: nodejs#35590
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants