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 13, 2020
Copy link
Member

@Trott Trott Oct 13, 2020

Choose a reason for hiding this comment

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

Suggested change
console.log(`Current groups: ${process.getgroups()}`);
console.log(process.getgroups()); // [ 16, 21, 297 ]

Embedding it in a string might make it less clear what the output is in this case. Raw array output might be better.

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 - I understand your point and thanks for the review. I have followed your suggestion. Please have a look.

PR-URL: nodejs#35625
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott Trott force-pushed the process-getgroups branch from 78cb167 to f59d4e0 Compare October 15, 2020 11:02
@Trott Trott merged commit f59d4e0 into nodejs:master Oct 15, 2020
@Trott
Copy link
Member

Trott commented Oct 15, 2020

Landed in f59d4e0

@PoojaDurgad I usually don't bother contributors with this detail, but because you are opening a lot of pull requests: Usually, only the first commit needs to be formatted according to our guidelines. For subsequent commits that are small changes to your pull requests (such as in this one where you had a second commit that removed extraneous whitespace and a third commit that implemented a small suggestion from a reviewer), it helps if you use git commit --fixup HEAD rather than writing a commit message. This will both let someone who lands the commit manually use --autosquash and it also means that your pull requests can be correctly landed by the bot we use (because it will run --autosquash if it can).

@PoojaDurgad
Copy link
Contributor Author

@Trott - Thanks for the suggestion. I will follow up.

BethGriggs pushed a commit that referenced this pull request Dec 8, 2020
PR-URL: #35625
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 10, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
PR-URL: nodejs#35625
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[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.

4 participants