Skip to content

Instantly share code, notes, and snippets.

@t3hcr
Forked from marionzualo/code_review.md
Created June 10, 2016 03:31
Show Gist options
  • Select an option

  • Save t3hcr/e1fb3e513af71931812a971eb26202d3 to your computer and use it in GitHub Desktop.

Select an option

Save t3hcr/e1fb3e513af71931812a971eb26202d3 to your computer and use it in GitHub Desktop.

Revisions

  1. @marionzualo marionzualo revised this gist May 25, 2015. 1 changed file with 5 additions and 0 deletions.
    5 changes: 5 additions & 0 deletions pull_request_template.md
    Original file line number Diff line number Diff line change
    @@ -1,6 +1,11 @@
    #### What's this PR do?

    #### Where should the reviewer start?

    #### How should this be manually tested?

    #### Any background context you want to provide?

    #### What are the relevant tickets?

    #### Screenshots (if appropriate)
  2. @marionzualo marionzualo revised this gist May 25, 2015. 1 changed file with 2 additions and 0 deletions.
    2 changes: 2 additions & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -81,6 +81,8 @@ Pro tip: Keep your own checklist of things you look for in a code review as in [
    # Links
    Links from places on the Internet that helped write this:

    (The most useful ones are [9], [1], [3] and [2].)

    * [1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    * [2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    * [3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
  3. @marionzualo marionzualo revised this gist May 25, 2015. 1 changed file with 15 additions and 12 deletions.
    27 changes: 15 additions & 12 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -1,17 +1,18 @@
    # Code Review
    A guide for reviewing code and having your code reviewed.

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    # Purpose
    Code review is an important part of a team's development process. It helps to:

    * disseminate knowledge about the codebase/technology/techniques across teams
    * increase awareness of the features being developed
    * provide opportunities to educate junior engineers (actually all engineers)
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs
    * develop a sense of team ownership of the code (no more his/her code)

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    When doing code reviews, **the process is as important as the result**, therefore, some guidelines are presented below to help improve the process of code review.

    # Everyone
    @@ -31,13 +32,13 @@ When doing code reviews, **the process is as important as the result**, therefor
    solution:" comments. Post a follow-up comment summarizing offline discussion.

    # Having Your Code Reviewed
    * Review your code before anybody else
    * Review your code before anybody else.
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. You only need to answer the relevant questions. Pro tip: use the bookmarklet [11] or chrome extension [12] for pull request description templates.
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Be grateful for the reviewer's suggestions. ("Good call. I'll make that
    change.")
    * Don't take it personally. The review is of the code, not you.
    * Don't take it personally. The review is about the code, not you.
    * Explain why the code exists. ("It's like that because of these reasons. Would
    it be more clear if I rename this class/file/method/variable?")
    * Seek to understand the reviewer's perspective.
    @@ -48,45 +49,47 @@ When doing code reviews, **the process is as important as the result**, therefor

    # Reviewing Code
    * Code review often and for short sessions. The effectiveness of code review drops after around an hour. Set aside time throughout your day to coincide with breaks in order to not disrupt your flow.
    * Review at least part of the code, even if you can't do all of it
    * Review the right things, let tools to do the rest. Don't focus on reviewing things that can be reviewed by tools (e.g. Rubocop, jsLint, sass lint, etc).
    * Review at least part of the code, even if you can't do all of it.
    * Review the right things, let tools to do the rest. Don't focus on reviewing things that can be reviewed by tools (e.g. Rubocop, jsLint, sass lint, etc). If the tools don't provide good feedback, change the config or improve the tools.

    Understand why the code is necessary (bug, user experience, refactoring). Then:

    * Communicate which ideas you feel strongly about and those you don't.
    * Identify ways to simplify the code while still solving the problem.
    * If discussions turn too philosophical or academic, move the discussion offline. In the meantime, let the
    author make the final decision on alternative implementations.
    * Offer alternative implementations, but assume the author already considered
    them. ("What do you think about using a custom validator here?")
    * Seek to understand the author's perspective.
    * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment.
    * Compliment/reinforce good practices
    * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment... always!
    * Compliment/reinforce good practices.

    Some suggestions of things to look for when reviewing code:

    * Is all the code easily understood? Is code unnecessarily complex ?
    * Can variable/method names be improved?
    * Are classes/files/methods becoming too large? Should they be split into smaller pieces?
    * Does it conform to the agreed coding conventions?
    * Is there any redundant or duplicate code? Is the code as modular as possible?
    * Could the design of the solution be improved (e.g. single responsibility principle, open/closed principle) ?
    * Is the code testable? (i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?)
    * Is the code testable? (i.e. don't add too many or hide dependencies, unable to initialize objects, test frameworks can use methods, do tests exist and are they comprehensive?)
    * Does the code work? Does it perform its intended function, the logic is correct etc.
    * Things related to your area of expertiese

    Pro tip: Keep your own checklist of things you look for in a code review as in [9] [7].

    # Links
    Links from places on the Internet that helped write this:

    * [1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    * [2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    * [3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
    * [4] some articles on how to write good commit messages - [first](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [second](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [third](https://wiki.openstack.org/wiki/GitCommitMessages)
    * [4] some articles on how to write good commit messages - [first](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html), [second](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [third](https://wiki.openstack.org/wiki/GitCommitMessages)
    * [5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [8] [Effective code review tips](http://blog.fogcreek.com/effective-code-reviews-9-tips-from-a-converted-skeptic/) - from Fog Creek Software
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
    * [10] [11 proven practices to improve code reviews](http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/) - nice article backed by data from studies.
    * [11] [Pull request template bookmarklet](https://quickleft.com/blog/pull-request-template-bookmarklet/)
    * [12] [Pull request template chrome extension](https://github.com/sprintly/pull-request-template-chrome-extension)
    * [12] [Pull request template chrome extension](https://github.com/sprintly/pull-request-template-chrome-extension)
  4. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 0 deletions.
    1 change: 1 addition & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -8,6 +8,7 @@ Code review is an important part of a team's development process. It helps to:
    * provide opportunities to educate junior engineers (actually all engineers)
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs
    * develop a sense of team ownership of the code (no more his/her code)

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
  5. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -5,7 +5,7 @@ A guide for reviewing code and having your code reviewed.
    Code review is an important part of a team's development process. It helps to:
    * disseminate knowledge about the codebase/technology/techniques across teams
    * increase awareness of the features being developed
    * provide opportunities to educate junior engineers
    * provide opportunities to educate junior engineers (actually all engineers)
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs

  6. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 2 additions and 1 deletion.
    3 changes: 2 additions & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -32,7 +32,7 @@ When doing code reviews, **the process is as important as the result**, therefor
    # Having Your Code Reviewed
    * Review your code before anybody else
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Pro tip: use the bookmarklet [11] or chrome extension [12] for pull request description templates.
    * Use the pull request description template [3]. You only need to answer the relevant questions. Pro tip: use the bookmarklet [11] or chrome extension [12] for pull request description templates.
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Be grateful for the reviewer's suggestions. ("Good call. I'll make that
    change.")
    @@ -71,6 +71,7 @@ Some suggestions of things to look for when reviewing code:
    * Is the code testable? (i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?)
    * Does the code work? Does it perform its intended function, the logic is correct etc.
    * Things related to your area of expertiese

    Pro tip: Keep your own checklist of things you look for in a code review as in [9] [7].

  7. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 2 additions and 1 deletion.
    3 changes: 2 additions & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -62,7 +62,8 @@ Understand why the code is necessary (bug, user experience, refactoring). Then:
    * Compliment/reinforce good practices

    Some suggestions of things to look for when reviewing code:
    * Is all the code easily understood? Can variable/method names be improved?
    * Is all the code easily understood? Is code unnecessarily complex ?
    * Can variable/method names be improved?
    * Are classes/files/methods becoming too large? Should they be split into smaller pieces?
    * Does it conform to the agreed coding conventions?
    * Is there any redundant or duplicate code? Is the code as modular as possible?
  8. @marionzualo marionzualo revised this gist May 23, 2015. No changes.
  9. @marionzualo marionzualo revised this gist May 23, 2015. 2 changed files with 7 additions and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -86,4 +86,4 @@ Links from places on the Internet that helped write this:
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
    * [10] [11 proven practices to improve code reviews](http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/) - nice article backed by data from studies.
    * [11] [Pull request template bookmarklet](https://quickleft.com/blog/pull-request-template-bookmarklet/)
    * [12] [Pull request template chrome extension](https://github.com/sprintly/pull-request-template-chrome-extension)
    * [12] [Pull request template chrome extension](https://github.com/sprintly/pull-request-template-chrome-extension)
    6 changes: 6 additions & 0 deletions pull_request_template.md
    Original file line number Diff line number Diff line change
    @@ -0,0 +1,6 @@
    #### What's this PR do?
    #### Where should the reviewer start?
    #### How should this be manually tested?
    #### Any background context you want to provide?
    #### What are the relevant tickets?
    #### Screenshots (if appropriate)
  10. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 2 additions and 2 deletions.
    4 changes: 2 additions & 2 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -11,7 +11,7 @@ Code review is an important part of a team's development process. It helps to:

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    When doing code reviews, *the process is as important as the result*, therefore, some guidelines are presented below to help improve the process of code review.
    When doing code reviews, **the process is as important as the result**, therefore, some guidelines are presented below to help improve the process of code review.

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
    @@ -32,7 +32,7 @@ When doing code reviews, *the process is as important as the result*, therefore,
    # Having Your Code Reviewed
    * Review your code before anybody else
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Or better, bookmarklet [11] or chrome extension [12].
    * Use the pull request description template [3]. Pro tip: use the bookmarklet [11] or chrome extension [12] for pull request description templates.
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Be grateful for the reviewer's suggestions. ("Good call. I'll make that
    change.")
  11. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -32,7 +32,7 @@ When doing code reviews, *the process is as important as the result*, therefore,
    # Having Your Code Reviewed
    * Review your code before anybody else
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Or better, bookmarklet [11] or crhome extension [12].
    * Use the pull request description template [3]. Or better, bookmarklet [11] or chrome extension [12].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Be grateful for the reviewer's suggestions. ("Good call. I'll make that
    change.")
  12. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 4 additions and 4 deletions.
    8 changes: 4 additions & 4 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -30,6 +30,10 @@ When doing code reviews, *the process is as important as the result*, therefore,
    solution:" comments. Post a follow-up comment summarizing offline discussion.

    # Having Your Code Reviewed
    * Review your code before anybody else
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Or better, bookmarklet [11] or crhome extension [12].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Be grateful for the reviewer's suggestions. ("Good call. I'll make that
    change.")
    * Don't take it personally. The review is of the code, not you.
    @@ -40,10 +44,6 @@ When doing code reviews, *the process is as important as the result*, therefore,
    * Wait to merge the branch until Continuous Integration
    tells you the test suite is green in the branch.
    * Merge once you feel confident in the code and its impact on the project.
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Or better, bookmarklet [11] or crhome extension [12].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Review your code before anybody else

    # Reviewing Code
    * Code review often and for short sessions. The effectiveness of code review drops after around an hour. Set aside time throughout your day to coincide with breaks in order to not disrupt your flow.
  13. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -11,7 +11,7 @@ Code review is an important part of a team's development process. It helps to:

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    When doing code reviews, the process is as important as the result, therefore, some guidelines are presented below to help improve the process.
    When doing code reviews, *the process is as important as the result*, therefore, some guidelines are presented below to help improve the process of code review.

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
  14. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -11,7 +11,7 @@ Code review is an important part of a team's development process. It helps to:

    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    When doing code reviews, the process is as important as the result, therefore, below some guidelines are presented to help improve the process.
    When doing code reviews, the process is as important as the result, therefore, some guidelines are presented below to help improve the process.

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
  15. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -9,7 +9,7 @@ Code review is an important part of a team's development process. It helps to:
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs

    > Peer code reviews are the single biggest thing you can do to improve your code - Jeff Atwood
    > Peer code reviews are the single biggest thing you can do to improve your code - [Jeff Atwood](http://blog.codinghorror.com/code-reviews-just-do-it/)
    When doing code reviews, the process is as important as the result, therefore, below some guidelines are presented to help improve the process.

  16. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 4 additions and 0 deletions.
    4 changes: 4 additions & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -9,6 +9,10 @@ Code review is an important part of a team's development process. It helps to:
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs

    > Peer code reviews are the single biggest thing you can do to improve your code - Jeff Atwood
    When doing code reviews, the process is as important as the result, therefore, below some guidelines are presented to help improve the process.

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
    you prefer, and reach a resolution quickly.
  17. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 5 additions and 3 deletions.
    8 changes: 5 additions & 3 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -3,9 +3,11 @@ A guide for reviewing code and having your code reviewed.

    # Purpose
    Code review is an important part of a team's development process. It helps to:
    + disseminate knowledge about the codebase/technology/techniques across teams
    + provide opportunities to educate junior engineers
    + catch poor architectural decisions and bugs
    * disseminate knowledge about the codebase/technology/techniques across teams
    * increase awareness of the features being developed
    * provide opportunities to educate junior engineers
    * find alternative solutions to problems
    * catch poor architectural decisions and bugs

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
  18. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -4,8 +4,8 @@ A guide for reviewing code and having your code reviewed.
    # Purpose
    Code review is an important part of a team's development process. It helps to:
    + disseminate knowledge about the codebase/technology/techniques across teams
    + catch poor architectural decisions and bugs
    + provide opportunities to educate junior engineers
    + catch poor architectural decisions and bugs

    # Everyone
    * Accept that many programming decisions are opinions. Discuss tradeoffs, which
  19. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 1 deletion.
    2 changes: 1 addition & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -72,7 +72,7 @@ Links from places on the Internet that helped write this:
    * [1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    * [2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    * [3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
    * [4] some articles on how to write good commit messages - [a](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [b](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [c](https://wiki.openstack.org/wiki/GitCommitMessages)
    * [4] some articles on how to write good commit messages - [first](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [second](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [third](https://wiki.openstack.org/wiki/GitCommitMessages)
    * [5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
  20. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 3 additions and 4 deletions.
    7 changes: 3 additions & 4 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -34,8 +34,8 @@ Code review is an important part of a team's development process. It helps to:
    * Wait to merge the branch until Continuous Integration
    tells you the test suite is green in the branch.
    * Merge once you feel confident in the code and its impact on the project.
    * Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
    * Use the pull request description template[3]. Or better, bookmarklet[11] or crhome extension[12].
    * Provide context about what the change is and why is it important - Write informative commit messages [4] and pull request descriptions.
    * Use the pull request description template [3]. Or better, bookmarklet [11] or crhome extension [12].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Review your code before anybody else

    @@ -67,8 +67,7 @@ Some suggestions of things to look for when reviewing code:

    Pro tip: Keep your own checklist of things you look for in a code review as in [9] [7].

    # Bibliography

    # Links
    Links from places on the Internet that helped write this:
    * [1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    * [2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
  21. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 4 additions and 2 deletions.
    6 changes: 4 additions & 2 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -35,7 +35,7 @@ Code review is an important part of a team's development process. It helps to:
    tells you the test suite is green in the branch.
    * Merge once you feel confident in the code and its impact on the project.
    * Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
    * Use the pull request description template[3].
    * Use the pull request description template[3]. Or better, bookmarklet[11] or crhome extension[12].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Review your code before anybody else

    @@ -79,4 +79,6 @@ Links from places on the Internet that helped write this:
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [8] [Effective code review tips](http://blog.fogcreek.com/effective-code-reviews-9-tips-from-a-converted-skeptic/) - from Fog Creek Software
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
    * [10] [11 proven practices to improve code reviews](http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/) - nice article backed by data from studies.
    * [10] [11 proven practices to improve code reviews](http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/) - nice article backed by data from studies.
    * [11] [Pull request template bookmarklet](https://quickleft.com/blog/pull-request-template-bookmarklet/)
    * [12] [Pull request template chrome extension](https://github.com/sprintly/pull-request-template-chrome-extension)
  22. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 3 additions and 1 deletion.
    4 changes: 3 additions & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -41,6 +41,7 @@ Code review is an important part of a team's development process. It helps to:

    # Reviewing Code
    * Code review often and for short sessions. The effectiveness of code review drops after around an hour. Set aside time throughout your day to coincide with breaks in order to not disrupt your flow.
    * Review at least part of the code, even if you can't do all of it
    * Review the right things, let tools to do the rest. Don't focus on reviewing things that can be reviewed by tools (e.g. Rubocop, jsLint, sass lint, etc).

    Understand why the code is necessary (bug, user experience, refactoring). Then:
    @@ -77,4 +78,5 @@ Links from places on the Internet that helped write this:
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [8] [Effective code review tips](http://blog.fogcreek.com/effective-code-reviews-9-tips-from-a-converted-skeptic/) - from Fog Creek Software
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
    * [10] [11 proven practices to improve code reviews](http://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/) - nice article backed by data from studies.
  23. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 3 additions and 1 deletion.
    4 changes: 3 additions & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -54,7 +54,7 @@ Understand why the code is necessary (bug, user experience, refactoring). Then:
    * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment.
    * Compliment/reinforce good practices

    Some suggestions of things to look for when reviewing code [9] [7]:
    Some suggestions of things to look for when reviewing code:
    * Is all the code easily understood? Can variable/method names be improved?
    * Are classes/files/methods becoming too large? Should they be split into smaller pieces?
    * Does it conform to the agreed coding conventions?
    @@ -64,6 +64,8 @@ Some suggestions of things to look for when reviewing code [9] [7]:
    * Do tests exist and are they comprehensive?)
    * Does the code work? Does it perform its intended function, the logic is correct etc.

    Pro tip: Keep your own checklist of things you look for in a code review as in [9] [7].

    # Bibliography

    Links from places on the Internet that helped write this:
  24. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 3 additions and 3 deletions.
    6 changes: 3 additions & 3 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -59,9 +59,9 @@ Some suggestions of things to look for when reviewing code [9] [7]:
    * Are classes/files/methods becoming too large? Should they be split into smaller pieces?
    * Does it conform to the agreed coding conventions?
    * Is there any redundant or duplicate code? Is the code as modular as possible?
    * Could the design of the solution be improved (e.g. Single responsibility principle, open/closed principle) ?
    * Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?
    * Could the design of the solution be improved (e.g. single responsibility principle, open/closed principle) ?
    * Is the code testable? (i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?)
    * Does the code work? Does it perform its intended function, the logic is correct etc.

    # Bibliography
  25. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 6 additions and 3 deletions.
    9 changes: 6 additions & 3 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -37,6 +37,7 @@ Code review is an important part of a team's development process. It helps to:
    * Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
    * Use the pull request description template[3].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.
    * Review your code before anybody else

    # Reviewing Code
    * Code review often and for short sessions. The effectiveness of code review drops after around an hour. Set aside time throughout your day to coincide with breaks in order to not disrupt your flow.
    @@ -51,12 +52,14 @@ Understand why the code is necessary (bug, user experience, refactoring). Then:
    them. ("What do you think about using a custom validator here?")
    * Seek to understand the author's perspective.
    * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment.
    * Compliment/reinforce good practices

    Some suggestions of things to look for when reviewing code[7]:
    * Is all the code easily understood?
    Some suggestions of things to look for when reviewing code [9] [7]:
    * Is all the code easily understood? Can variable/method names be improved?
    * Are classes/files/methods becoming too large? Should they be split into smaller pieces?
    * Does it conform to the agreed coding conventions?
    * Is there any redundant or duplicate code? Is the code as modular as possible?
    * Could the design of the solution be improved?
    * Could the design of the solution be improved (e.g. Single responsibility principle, open/closed principle) ?
    * Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?
    * Does the code work? Does it perform its intended function, the logic is correct etc.
  26. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 0 deletions.
    1 change: 1 addition & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -72,3 +72,4 @@ Links from places on the Internet that helped write this:
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [8] [Effective code review tips](http://blog.fogcreek.com/effective-code-reviews-9-tips-from-a-converted-skeptic/) - from Fog Creek Software
    * [9] [Code Review Best Practices](http://kevinlondon.com/2015/05/05/code-review-best-practices.html) - really good article on code review best practices
  27. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 3 additions and 0 deletions.
    3 changes: 3 additions & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -39,6 +39,8 @@ Code review is an important part of a team's development process. It helps to:
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.

    # Reviewing Code
    * Code review often and for short sessions. The effectiveness of code review drops after around an hour. Set aside time throughout your day to coincide with breaks in order to not disrupt your flow.
    * Review the right things, let tools to do the rest. Don't focus on reviewing things that can be reviewed by tools (e.g. Rubocop, jsLint, sass lint, etc).

    Understand why the code is necessary (bug, user experience, refactoring). Then:
    * Communicate which ideas you feel strongly about and those you don't.
    @@ -69,3 +71,4 @@ Links from places on the Internet that helped write this:
    * [5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [8] [Effective code review tips](http://blog.fogcreek.com/effective-code-reviews-9-tips-from-a-converted-skeptic/) - from Fog Creek Software
  28. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 1 addition and 0 deletions.
    1 change: 1 addition & 0 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -36,6 +36,7 @@ Code review is an important part of a team's development process. It helps to:
    * Merge once you feel confident in the code and its impact on the project.
    * Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
    * Use the pull request description template[3].
    * Annotate your code reviews. Add comments anywhere that you feel will help the reviewer understand your code.

    # Reviewing Code

  29. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 7 additions and 7 deletions.
    14 changes: 7 additions & 7 deletions code_review.md
    Original file line number Diff line number Diff line change
    @@ -61,10 +61,10 @@ Some suggestions of things to look for when reviewing code[7]:
    # Bibliography

    Links from places on the Internet that helped write this:
    *[1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    *[2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    *[3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
    *[4] some articles on how to write good commit messages - [a](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [b](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [c](https://wiki.openstack.org/wiki/GitCommitMessages)
    +[5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    +[6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    +[7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
    * [1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    * [2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    * [3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
    * [4] some articles on how to write good commit messages - [a](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [b](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [c](https://wiki.openstack.org/wiki/GitCommitMessages)
    * [5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    * [6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    * [7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software
  30. @marionzualo marionzualo revised this gist May 23, 2015. 1 changed file with 21 additions and 1 deletion.
    22 changes: 21 additions & 1 deletion code_review.md
    Original file line number Diff line number Diff line change
    @@ -34,11 +34,12 @@ Code review is an important part of a team's development process. It helps to:
    * Wait to merge the branch until Continuous Integration
    tells you the test suite is green in the branch.
    * Merge once you feel confident in the code and its impact on the project.
    * Provide context about what the change is and why is it important - Write informative commit messages[4] and pull request descriptions.
    * Use the pull request description template[3].

    # Reviewing Code

    Understand why the code is necessary (bug, user experience, refactoring). Then:

    * Communicate which ideas you feel strongly about and those you don't.
    * Identify ways to simplify the code while still solving the problem.
    * If discussions turn too philosophical or academic, move the discussion offline. In the meantime, let the
    @@ -48,3 +49,22 @@ Understand why the code is necessary (bug, user experience, refactoring). Then:
    * Seek to understand the author's perspective.
    * Sign off on the pull request with a :thumbsup: or "Ready to merge" comment.

    Some suggestions of things to look for when reviewing code[7]:
    * Is all the code easily understood?
    * Does it conform to the agreed coding conventions?
    * Is there any redundant or duplicate code? Is the code as modular as possible?
    * Could the design of the solution be improved?
    * Is the code testable? i.e. don’t add too many or hide dependencies, unable to initialize objects, test frameworks can use methods etc.
    * Do tests exist and are they comprehensive?
    * Does the code work? Does it perform its intended function, the logic is correct etc.

    # Bibliography

    Links from places on the Internet that helped write this:
    *[1] [Talk - Implementing a Strong Code-Review culture](https://www.youtube.com/watch?v=PJjmw9TRB7s) - the talk that inspired this.
    *[2] [Thoughtbot's Guide for Code-Review](https://github.com/thoughtbot/guides/blob/master/code-review/README.md) - straight to the point guidelines on code review. A lot of the content was taken from there.
    *[3] [Pull request templates make code review easier](https://quickleft.com/blog/pull-request-templates-make-code-review-easier/) - introduction to pull request templates.
    *[4] some articles on how to write good commit messages - [a](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) [b](http://www.slideshare.net/TarinGamberini/commit-messages-goodpractices), [c](https://wiki.openstack.org/wiki/GitCommitMessages)
    +[5] [Am I My Code?](http://mfeckie.github.io/Am-I-My-Code/) - this article explores some strategies for giving feedback when reviewing code.
    +[6] [Code review at the Dropbox iOS team](http://www.objc.io/issue-22/dropbox.html) - the article goes over their whole process of review. It has some interesting ideas.
    +[7] [Code Review Checklist](http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) - from Fog Creek Software