Code Review as a Collaborative Journey

Code Review as a Collaborative Journey

Code review is not just an inspection but rather a journey of the reviewer and the reviewee working together to reach the goal - delivering a good product.

In this talk, we are explaining about how a reviewer can make a collaborative code review, especially on Android app development. Android app development requires the reviewer to take into account various considerations; while there are some useful tips for the reviewer to find out potential bugs or improvements in the code.

First, we define what a good code review is and explain the typical steps of a good code review. We think a typical process of a good code review is as follows:

1. Understand the goal and the background of the change
2. Run the app and verify the behaviour
3. Read the code and the layout XML carefully
4. Add any comments you come up with in your mind
5. Look back on your review
6. Have a conversation with the reviewee

Then we will dive into each step and show some key tips for the review of Android development with an example from our real experience. For example, we believe running the app on a device is crucial to the review of the Android app code, so we will give an example and explain why it is important.

Instead of talking about important yet too general things around code review, such as wordings of the comments, whether or not pointing out a format, etc., we are rather focusing on the entire process and Android-specific topics.

979d93b360f80486b121486a9d063ad5?s=128

Hiroshi Kurokawa

February 08, 2019
Tweet

Transcript

  1. Code Review as a Collabora/ve Journey Hiroshi Kurokawa, Takafumi Nanao,

    mixi, Inc., DroidKaigi 2019
  2. What is Code Review For?

  3. What is Code Review NOT For? • Finding out a

    programming bug • Code forma3ng, lin6ng ♻ • Discussing the specifica6on #
  4. Code Review is for Collabora0on • Sharing knowledge ! •

    Sharing skill " • Making a good product through discussion
  5. What Informa,on is required for Code Review? • Goal •

    Background • Strategy • Changes made • Verifica:on steps
  6. None
  7. The Story of Code Review Photo by Annie Spra0 on

    Unsplash
  8. 1. Understand the Background and the Goal Photo by Christopher

    Burns on Unsplash
  9. 2. Go through the Strategy and What was Made Photo

    by Annie Spra0 on Unsplash
  10. 3. Verify if the Change Really Takes You to the

    Goal Photo by delfi de la Rua on Unsplash
  11. 4. Look into Every Changes One by One Photo by

    Caleb Jones on Unsplash
  12. 5. Look Back on the Change from the Goal Photo

    by Vlad Bagacian on Unsplash
  13. 6. Have a Conversa-on with the Reviewee Photo by rawpixel

    on Unsplash
  14. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made 3. Verify if the change really takes you to the goal 4. Look into every changes one by one 5. Look back on the change from the goal 6. Have a conversaDon with the reviewee Photo by Annie Spra0 on Unsplash
  15. 1. Understand the Background and the Goal

  16. Understanding the background • Read the descrip.on • Are enough

    details wri6en to review it or is it using pull request template?
  17. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made
  18. Strategy • organize informa-on • review the strategy Photo by

    Jamie Street on Unsplash
  19. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made 3. Verify if the change really takes you to the goal
  20. Opera&onal check Checkout the branch and build it locally. The

    git worktree command is very useful. git worktree add ~/code-review/branch_name branch_name android-studio.sh ~/code-review/branch_name
  21. Build • Warnings in build log or logcat • Try

    proguard build or release build when relevant file was changed Photo by Randy Fath on Unsplash
  22. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made 3. Verify if the change really takes you to the goal 4. Look into every changes one by one
  23. UI • Is it along the guidelines of Material Design

    or your team's? • Is it not like iOS UI? • Is it using anima<on properly? • Change anima<on scale • Does scrolling views work smoothly? Photo by rawpixel on Unsplash
  24. UI behind UI • Is it handling errors? • Does

    it work even with no data? • add debug code directly • Airplane mode Photo by Mikhail Vasilyev on Unsplash
  25. Lifecycle • Does it work a-er returning from another screen?

    • Does it work a-er screen rota6on? • Does it work a-er the process was killed? • Does it manage the Disposable or Closable at the right 6ming • Can it handle bu<ons being tapped very fast? Photo by Robert Anasch on Unsplash
  26. Performance • Is working fast? 1s is slow. • Does

    it save ba7ery? • Doesn't any crash happen? Photo by Jonathan Chng on Unsplash
  27. Read code • use Android Studio • If you have

    ques5on, don't hesitate to check actual opera5on. • Is there any newly unnecessary code by that change. Photo by Maximilian Weisbecker on Unsplash
  28. Layout • unnecessary stuffs • unnecessary nest • deprecated components

    Photo by Jose Alejandro Cuffia on Unsplash
  29. Code format • share your coding rules • forma3ng •

    rules of lint # .gitignore !.idea/codeStyles !.idea/inspectionProfiles Photo by Fabio Santaniello Bruun on Unsplash
  30. 3rd party libraries • Is it trustworthy? • Is it

    maintained well? • Are there be6er alterna7ves? Photo by Glen Noble on Unsplash
  31. Leaks • Use it watching the profiler of AS •

    Does it have a strong reference to Ac8vity? • Does it close Closable? Photo by Luis Tosta on Unsplash
  32. Tests • Are proper tests added? • Are tests enough

    for addi3onal condi3ons or boundary values? Photo by Nicolas Thomas on Unsplash
  33. Side effects • Adding Permission • Moving Applica4on class Photo

    by rawpixel on Unsplash
  34. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made 3. Verify if the change really takes you to the goal 4. Look into every changes one by one 5. Look back on the change from the goal
  35. Look back • Check if your opinion or recogni1on is

    right. • Imagine what you would do if you were the reviewee.
  36. 1. Understand the Background and the Goal 2. Go through

    the strategy and what was made 3. Verify if the change really takes you to the goal 4. Look into every changes one by one 5. Look back on the change from the goal 6. Have a conversa+on with the reviewee
  37. Values We should share common values in our team. •

    Easy to read and change • Proper package name Photo by Lina Trochez on Unsplash
  38. Give your feedback • Feedback details to improve the code

    • Use code sugges7on • Tell condi7ons and steps to repro bug • Compliments are welcome
  39. FIN