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.

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
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