Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Code Review 101

Code Review 101

In the open source environment, any bug fix, new feature, or documentation goes over a review,
where the project maintainers has the opportunity to ask and suggest improvements before the
change get integrated into the project. This practice has many benefits that’s why many companies
incorporated this practice in their development flow.

In this talk, we’ll discuss:

* What’s code review and Pull/Merge requests
* Why use code reviews
* When you should ask for a code review
* Code Review vs Pair Programming
* Good and bad practices
* Code Review in development/deployment flow

After this talk, you’ll learn why code reviews are important and how to extract the best of this practice.

50e713934ed341675bf1fa73127ec260?s=128

Ulisses Almeida

April 18, 2019
Tweet

Transcript

  1. Code Review 101 Ulisses Almeida

  2. Code Review CODE REVIEW 101 What is it? Options Why

    Pull Requests? Pull requests good practices Go beyond with pull requests
  3. Ulisses Almeida CODE REVIEW 101 @ulissesalmeida
 
 Elixir Developer
 Payments

    Team
  4. What’s Code Review?

  5. Bunch of folks reading code and suggesting changes W E

    L C O M E A B O A R D Improve code quality, prevent defects, share knowledge and responsibility, discover better solutions
  6. None
  7. Options of code review

  8. Continuous
 Integration =~
 not having a code review CODE REVIEW

    101 Don’t branch! Don’t branch! Commits on master everyday! Feature toggle to hide WIP Automated tests Automated deployment Continuous Integration 
 != Continuous Delivery
 != Continuous Deployment https://martinfowler.com/articles/continuousIntegration.html
  9. Pair programming, kinda code review CODE REVIEW 101 Driver and

    Navigator Constant feedback during the coding process Faster on-boarding Focus No personal space, can be exhausting Lack of feedbacks from third team members
  10. Code Review CODE REVIEW 101 Synchronous vs Asynchronous

  11. Code Review, Merge/Pull requests CODE REVIEW 101 Open collaboration Time-zone/remote

    friendly History of discussions Enforce code quality before mainline integration Wait for review can take a long time Written communication is hard
  12. Why Pull Requests?

  13. First, automation always W E L C O M E

    A B O A R D Automated tests, deploys, rollbacks are important, no matter the process
  14. Continuous integration Pair programming Pull Requests Teammates early feedback 0

    ++ + Team Visibility - + ++ Team ownership - + ++ Individual space + - + Focus 0 ++ 0 Code Quality 0 + ++ Deploy confidence ++ + 0 ++ Strong promotes + Promotes 0 Neutral - Negates CODE REVIEW 101
  15. CODE REVIEW 101 Pair Programming Code 
 Review Continuous 


    Integration
  16. Good practices

  17. Creating a pull request

  18. Small Concise, Focused CODE REVIEW 101 Few code to read

    leads to a better quality of the review Give lots of information with few words and image Efforts to one goal
  19. CODE REVIEW 101 https://twitter.com/iamdevloper/status/397664295875805184

  20. CODE REVIEW 101 Quality of Review 
 Size of Pull

    Request
  21. Concise PR,
 is about optimise the reviewer time CODE REVIEW

    101 Clear title Good description about you have done and why You can use screenshots and GIFs Add Issue Tracker links (JIRA for example) Add reviewers Review your own PR and highlight some parts
  22. CODE REVIEW 101 • CloudApp • LiICEcap

  23. CODE REVIEW 101

  24. CODE REVIEW 101

  25. CODE REVIEW 101

  26. A focused PR starts before coding CODE REVIEW 101 Breakdown

    your user stories Try to delivery a testable thing But don’t break too much otherwise your PR will be pointless Work with your team to check if the tasks/stories are clear
  27. CODE REVIEW 101 0 27,5 55 82,5 110 #1 #2

    #3 #4 Backlog Developing Review Testing Done
  28. 0 27,5 55 82,5 110 #1 #2 #3 #4 Backlog

    Developing Review Testing Done CODE REVIEW 101
  29. CODE REVIEW 101 Pull Request Managers
 Stakeholders
 Developers
 QAs

  30. Big review times are symptoms W E L C O

    M E A B O A R D Removing pull requests might not cure the disease.
 
 Analyse the context:
 WIP? Big demands? Context switching?
  31. Reviewing a pull request

  32. CODE REVIEW 101 ⓵ ⓶ ⓷ ⓸

  33. What we 
 should look 
 for? CODE REVIEW 101

    Is it solving the problem? Is there other ways? Typos, bugs, security, performance Need tests? Documentation? Following the team/project patterns?
  34. CODE REVIEW 101 Start a discussion!

  35. Try to be assertive,
 written communication is hard CODE REVIEW

    101 Avoid imperative sentences Suggestive tone Teach, show examples Don't assume, ask questions Compliments Celebrate! Have fun!
  36. CODE REVIEW 101

  37. CODE REVIEW 101

  38. Receiving a review

  39. The code is 
 not you and 
 not just

    
 for you CODE REVIEW 101 Appreciate the reviewer time Assertive communication Answer the suggestions, even those you will not apply Lots of comments doesn’t mean you are bad programmer Detach from your code, it’s ok sometimes rewrite everything
  40. There are people behind the screens W E L C

    O M E A B O A R D Go beyond the tool.
 Face-to-face, video calls…
 
 Don’t forget to register the decision in PR later
  41. Go beyond with Pull Requests

  42. Automate the review CODE REVIEW 101

  43. Review apps CODE REVIEW 101

  44. Go crazy, and deploy the PR in production! CODE REVIEW

    101 https://guides.github.com/introduction/flow/
  45. Wrapping
 up!

  46. Code Review ensure quality of the code before integration

  47. Pull requests are great to have discussions with code context

    and history
  48. Following some good practices, it can be fast, fair and

    fun
  49. Thanks! Ulisses Almeida