Created
February 28, 2014 22:07
-
-
Save dcmoore/9281018 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