Code Review @ AppsFlyer

Transcript

1. Code Review Why & How Ran Tavory, Ido Barkan AppsFlyer Aug 2018
2. Code Review Why?
3. • For our Customers • For Us
4. For our customers The immune system “Running Continuous Deployment without an Immune System is playing Russian Roulette”
5. The Immune System What is the Immune System? • The line of defence will always be broken => Solution: Multiple lines of defence
6. The Immune System
7. The Immune System Code Review is part of the immune system • Coding ◦ Code Review ◦ ... • Monitoring ◦ ... • Instrumentation ◦ ... • KPI Monitoring ◦ ...
8. Code review - For us • Improve our skill ◦ Best way to Teach ◦ Best way to Learn • Maintain Coding conventions • Find Defects early • More devs familiar with the code
9. Programs must be written for people to read - Harold Abelson But if we don’t test code readability, how do we know it is readable?
10. Quality v/s Speed ? NO! Quality is a requirement in order to maintain speed WHY?
11. Exponential Value (and complexity)
12. Code Review How?
13. Committer The committer’s flow 1. Create a branch 2. Push to GitLab 3. Create Merge Request 4. Assign to approver (single one) 5. … back and forth 6. Approval 7. Merge
14. Reviewer The reviewer flow 1. Be informed 2. Review Code 3. Leave Comments 4. … back and forth 5. Approve when Happy
15. Async Code Review Async is preferred • Code review is Async by default ◦ The true code reading ◦ Remote devs • Move to Sync when not converging ◦ Only in rare cases ◦ Document your discussion
16. Code Review Flow (Commiter)
17. Code Review Flow (Commiter)
18. Code Review Flow (Reviewer)
19. GitLab - Default Project Settings
20. Code Review Best Practices
22. What to look for in code review • Keep away from checklists… But… • What I usually look for: ◦ I understand ◦ It makes sense ◦ Aesthetics (naming, style etc) ◦ SW Engineering (modularity, coherence, DRY etc) ◦ Docs ◦ Tests
23. Tips for successful review • Play nice. Don’t hurt egos • Be professional. It isn’t personal ◦ Concentrate on the code, not the personality • Understand, not criticize • Get prepared ◦ Technology ◦ Product ◦ Coding conventions ◦ Related code • Read slow • Short code reviews
24. How short? What size is ideal?
25. What size is ideal? • 100 - 300 LoC (research on C code, for Clojure - shorter) • 30-60 minutes
26. Styleguides • Clojure: https://github.com/bbatsov/clojure-style-guide • Javascript: https://github.com/airbnb/javascript • Go: https://github.com/golang/go/wiki/CodeReviewComments
27. Not Covered Today • Linters • Tests and Coverage • CI with CR • Design Review • Readability and Ownership • CR Checklists (?)
28. Thank you
29. References • This all started here https://community.appsflyer.com/t/code-review-practices/94 • https://smartbear.com/SmartBear/media/pdfs/best-kept-secrets-of-peer-code-r eview.pdf