Forked from dcmoore/Effectively Reviewing a Pull Request
Last active
August 29, 2015 14:17
-
-
Save thefonso/1df81048a28266a5225b to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Effectively Reviewing a Pull Request | |
==================================== | |
How do they work? | |
----------------- | |
- code on a branch or fork | |
- open pull request | |
- have people review code | |
- potentially update the code | |
- merge pull request | |
- profit | |
Why do pull requests? | |
--------------------- | |
- collective ownership | |
- consistent codebase | |
- find bugs before they're deployed | |
Best practices for reviewers | |
---------------------------- | |
- pull down code | |
- test code changes locally | |
- easier to see changes outside of a diff | |
- you're reviewing correctness, not style | |
- for minor changes, make them yourself | |
instead of commenting | |
- don't rush a review | |
- don't make it personal | |
- comments should be conversational not | |
accusational | |
- discussion should be focused on the code, | |
not the coder | |
- praise things that look good in addition | |
to constructive criticism | |
- don't leave pull requests hanging | |
- review at beginning and end of day or | |
during a break | |
- never let a PR go unreviewed for more than | |
24 hours | |
- as the reviewer it's not your way or the | |
highway | |
Best practices for reviewees | |
---------------------------- | |
- put a descriptive summary on each pull | |
request | |
- respond to each comment unless the reviewer | |
otherwise specifies | |
- don't get offended | |
- avoid massive code changes if possible |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment