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.

Sign up for GitHub

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

Jump to bottom

meta: articles about PR communication #16359

Closed
vsemozhetbyt opened this issue Oct 21, 2017 · 8 comments
Closed

meta: articles about PR communication #16359

vsemozhetbyt opened this issue Oct 21, 2017 · 8 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. meta Issues and PRs related to the general management of the project.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Oct 21, 2017 •
edited
Loading

Recently, I've come across these articles:

  1. A concise one, concerning more the PR author side:

https://slack.engineering/on-empathy-pull-requests-979e4257d158

  1. An elaborate one, concerning more the PR reviewer side:

https://mtlynch.io/human-code-reviews-1/
https://mtlynch.io/human-code-reviews-2/

I wonder if it is worth to find a place for referencing them in the CONTRIBUTING.md and COLLABORATOR_GUIDE.md respectively (or in some other more appropriate doc).

P.S. A new one: https://css-tricks.com/code-review-etiquette/
@vsemozhetbyt vsemozhetbyt added the meta Issues and PRs related to the general management of the project. label Oct 21, 2017
@apapirovski
Copy link
Contributor

I really like the second one. 👍 The first is a bit heavy handed about putting most of the responsibility on the person opening the PR. Maybe that's applicable to the culture at Slack but it certainly shouldn't be applicable to Node, especially given the number of first-time contributors.

@gibfahn gibfahn added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Oct 22, 2017
@gibfahn
Copy link
Member

gibfahn commented Oct 22, 2017

I wonder if it is worth to find a place for referencing them in the CONTRIBUTING.md and COLLABORATOR_GUIDE.md respectively (or in some other more appropriate doc).

SGTM, sounds like a good PR.

@sreepurnajasti
Copy link
Contributor

sreepurnajasti commented Oct 26, 2017 •
edited
Loading

@vsemozhetbyt Both the articles are good to connect. Feels like CONTRIBUTING.md (Pull Request section) a good fit to hold this.

@vsemozhetbyt
Copy link
Contributor Author

A new one: https://css-tricks.com/code-review-etiquette/

@vsemozhetbyt
Copy link
Contributor Author

Part 2: https://mtlynch.io/human-code-reviews-2/

@mhdawson
Copy link
Member

+1 to a reference in the collaborators guide.

@bnoordhuis
Copy link
Member

COLLABORATOR_GUIDE.md is 10 or 15 pages long already. The longer you make it, the less people will read or remember it all.

@SalameWilliam
Copy link

I'll try to tackle this and include them in the CONTRIBUTING.md and/or the COLLABORATOR_GUIDE.md

SalameWilliam pushed a commit to SalameWilliam/node that referenced this issue Dec 28, 2017
doc: added articles about PR communication in CONTRIBUTING.md
1fb9fd7 
Added some references to PR communication articles in
Helpful Ressources inside CONTRIBUTING.md

Fixes: nodejs#16359
@SalameWilliam SalameWilliam mentioned this issue Dec 28, 2017
Closed
2 tasks
MylesBorins pushed a commit that referenced this issue Jan 8, 2018
@MylesBorins
doc: add references to PR communication articles
3fcf073 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Jan 9, 2018
@MylesBorins
doc: add references to PR communication articles
d89cce5 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Jan 9, 2018
@MylesBorins
doc: add references to PR communication articles
2f7030d 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2018
@MylesBorins
doc: add references to PR communication articles
68235da 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2018
@MylesBorins
doc: add references to PR communication articles
231c3bb 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Feb 11, 2018
@MylesBorins
doc: add references to PR communication articles
aea3b74 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Feb 12, 2018
@MylesBorins
doc: add references to PR communication articles
72b8924 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
MylesBorins pushed a commit that referenced this issue Feb 13, 2018
@MylesBorins
doc: add references to PR communication articles
21c5d82 
Added some references to PR communication articles in
Helpful Ressources inside COLLABORATOR_GUIDE.md

PR-URL: #17902
Fixes: #16359
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@apapirovski @bnoordhuis @mhdawson @vsemozhetbyt @gibfahn @SalameWilliam @sreepurnajasti