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

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.

Hiroshi Kurokawa

February 08, 2019
Tweet

More Decks by Hiroshi Kurokawa

Other Decks in Technology

Transcript

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

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

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

    Background • Strategy • Changes made • Verifica:on steps
  4. 3. Verify if the Change Really Takes You to the

    Goal Photo by delfi de la Rua on Unsplash
  5. 5. Look Back on the Change from the Goal Photo

    by Vlad Bagacian on Unsplash
  6. 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
  7. Understanding the background • Read the descrip.on • Are enough

    details wri6en to review it or is it using pull request template?
  8. 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
  9. 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
  10. Build • Warnings in build log or logcat • Try

    proguard build or release build when relevant file was changed Photo by Randy Fath on Unsplash
  11. 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
  12. 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
  13. 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
  14. 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
  15. Performance • Is working fast? 1s is slow. • Does

    it save ba7ery? • Doesn't any crash happen? Photo by Jonathan Chng on Unsplash
  16. 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
  17. Code format • share your coding rules • forma3ng •

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

    maintained well? • Are there be6er alterna7ves? Photo by Glen Noble on Unsplash
  19. 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
  20. Tests • Are proper tests added? • Are tests enough

    for addi3onal condi3ons or boundary values? Photo by Nicolas Thomas on Unsplash
  21. 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
  22. Look back • Check if your opinion or recogni1on is

    right. • Imagine what you would do if you were the reviewee.
  23. 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
  24. Values We should share common values in our team. •

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

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