Skip to content

Conversation

@refack
Copy link
Contributor

@refack refack commented Oct 20, 2017

Checklist
Affected core subsystem(s)

doc,meta

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 20, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

recommanded -> recommended ?

Copy link
Member

Choose a reason for hiding this comment

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

You could also drop the first four words:

Be extra welcoming to "first-time-contributors".

I'd use especially in place of extra and lose the unnecessary quotes and hyphens:

Be especially welcoming to first time contributors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. recommanded -> recommended ?
  2. Avoid "you"?
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove the "it is recommended that you"?:

For first time contributors, ask the author if they have configured...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack

Copy link
Member

Choose a reason for hiding this comment

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

Not an objection, but just a comment that I try to be very sparing with bold, italics, etc. It gets out of hand pretty easily.

Copy link
Member

Choose a reason for hiding this comment

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

linking -> liking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack

Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM modulo spelling nits.

Copy link
Member

Choose a reason for hiding this comment

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

minor nit: blank line before this sentence?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack

@refack refack force-pushed the collab-guide-tweaks branch from bb62506 to f5c8192 Compare October 23, 2017 11:33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not an objection, but just a comment that I try to be very sparing with bold, italics, etc. It gets out of hand pretty easily.

Ack. I felt this is a point worth emphasising both to Collaborators, and to contributors who read this (as on most screens it appears above the fold)

@refack
Copy link
Contributor Author

refack commented Oct 23, 2017

Addressed comment PTAL

@refack refack force-pushed the collab-guide-tweaks branch from f5c8192 to a9de89a Compare October 24, 2017 14:25
PR-URL: nodejs#16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@refack refack merged commit 7671f25 into nodejs:master Oct 24, 2017
@refack refack deleted the collab-guide-tweaks branch October 24, 2017 14:27
cjihrig pushed a commit that referenced this pull request Oct 25, 2017
PR-URL: #16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@cjihrig cjihrig mentioned this pull request Oct 25, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
PR-URL: nodejs/node#16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
PR-URL: #16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
PR-URL: #16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
PR-URL: #16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
PR-URL: nodejs/node#16350 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
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.

10 participants