Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: added articles about PR communication in CONTRIBUTING.md #17902

Closed
wants to merge 3 commits into from

Conversation

SalameWilliam
Copy link

Added some references to PR communication articles in
Helpful Ressources inside CONTRIBUTING.md

Fixes: #16359

Checklist
Affected core subsystem(s)

doc, meta

Added some references to PR communication articles in
Helpful Ressources inside CONTRIBUTING.md

Fixes: nodejs#16359
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 28, 2017
CONTRIBUTING.md Outdated
@@ -828,6 +828,8 @@ The following additional resources may be of assistance:
* [How to create a Minimal, Complete, and Verifiable example](https://stackoverflow.com/help/mcve)
* [core-validate-commit](https://github.com/evanlucas/core-validate-commit) -
A utility that ensures commits follow the commit formatting guidelines.
* How to respectfully and usefully review code part [one](https://mtlynch.io/human-code-reviews-1/) and [two](https://mtlynch.io/human-code-reviews-2/)
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Dec 28, 2017

Choose a reason for hiding this comment

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

I may be wrong, but do we need a comma or a colon between "code" and "part" here?

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Thank you! I have a nit question, but it can be resolved on landing the PR)

@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Dec 28, 2017
@Trott
Copy link
Member

Trott commented Dec 28, 2017

Hi, @vixwilli! Welcome, and thanks for the pull request. I have some questions about whether there might be a better place for this information, but I do appreciate the links!

@vsemozhetbyt or anyone else reviewing: Since these linked docs are about reviewing code, don't they more properly belong in the COLLABORATOR_GUIDE.md and not CONTRIBUTING.md?

General outlook from me: Our CONTRIBUTING.md is already waaaaaay too long and I'm reluctant to add anything that isn't absolutely essential. Ref: #17842

@vsemozhetbyt
Copy link
Contributor

@Trott Just a thought: our contributors sometimes also make various brief code notes for each other, so maybe these articles can be helpful for them too. However, I have no strong opinion about the place, so let us hear what others think)

Added a forgotten comma inside CONTRIBUTING.md
@gibfahn
Copy link
Member

gibfahn commented Dec 28, 2017

@vsemozhetbyt or anyone else reviewing: Since these linked docs are about reviewing code, don't they more properly belong in the COLLABORATOR_GUIDE.md and not CONTRIBUTING.md?

If they're primarily for reviewers, then collaborator guide makes more sense IMO.

@starkwang
Copy link
Contributor

+1 on moving this link to the COLLABORATOR_GUIDE.md because the most reviewers of our PR are collaborators.

@SalameWilliam
Copy link
Author

Where do you think I could put the links inside COLLABORATOR_GUIDE.md ? I initially wanted to put it there but since the guide is more about the technical side of things i didn't really know where to insert it. CONTRIBUTING.md already contains a more "human" side of PR like the articles mentionned and a Helpful Ressources section was already present so it was 'easier' to mention it there

Moved a few links about PR communication from CONTRIBUTING.md
to COLLABORATOR_GUIDE.md
@SalameWilliam
Copy link
Author

I placed the links inside COLLABORATOR_GUIDE.md in a specific section since the doc is quite long already and I didn't want to clutter the text with them. Tell me if you see a better spot to link them !

@Trott
Copy link
Member

Trott commented Dec 29, 2017

I placed the links inside COLLABORATOR_GUIDE.md in a specific section since the doc is quite long already and I didn't want to clutter the text with them. Tell me if you see a better spot to link them !

I like that location. 👍

@vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt
Copy link
Contributor

As it appears to be a bit controversial addition, let's wait till Monday before landing, lest anybody have some objections.

vsemozhetbyt pushed a commit that referenced this pull request Jan 2, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
@vsemozhetbyt
Copy link
Contributor

Landed in d179cca

Thank you, @vixwilli!

MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 10, 2018
MylesBorins pushed a commit that referenced this pull request Jan 24, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Weijia Wang <[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. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants