Code Reviews

Transcript

1. code.reviews Janos Gyerik correct clean efficient tested efficient efficient clean

   clean tested tested correct correct correct tested correc orrect correc tested efficient clean

2. 2 what is it?

3. 3 commit
 commit
 commit
 review
 commit
 commit
 review
 commit
 accept

   commit
 commit
 commit
 
 NOT code reviewed code reviewed

4. 4 why do it?

5. 5 UCXGVKOG catch early
 catch early catch early bugs bad

   patterns ugliness

6. 6

7. 7 peer review peer pressure

8. 8 anything that gets reviewed gets better

9. 9 quality time savings
 cost savings FACT

10. 10 correct readable efficient tested buggy messy crappy untested NOT

    code reviewed code reviewed

11. 11 information sharing NO MORE
 bottleneck developers

12. 12

13. 13 why/when
 NOT do it?

14. 14 big bang development too hard to control should be

    the exception, not the norm! code reviews 
 are not practical when…

15. 15 without supporting tools tools help doing code reviews efficiently

    don’t waste your time, get them and use them! code reviews 
 are not practical…

16. 16 what’s required? disciplined commits

17. 17 incremental changes small and stable logical steps at all

    levels

18. 18 one feature one purpose one branch feature

19. 19 short-lived max 3 days feature

20. 20 (example good branch)

21. 21 (example bad branch)

22. 22 one commit one logical change

23. 23 one good commit == stable build; related changes; no

    garbage; small; good comment

24. 24

25. 25 (example good commit)

26. 26 example bad commit with many changes)

27. 27 (example bad commit with garbage)

28. 28 how to do it?

29. 29 git fetch origin master git checkout -b feature-x origin/ma

    # work work work git commit git commit git push origin feature-x # create merge request

30. 30 Create Merge Request create; don’t assign! self-review: any WTFs?

    ready? assign!

31. 31 peer review peer pressure

32. 32 what to review?

33. 33 readable is it clear? easy to read? easy to

    understand? FACT Code is read far more often
 than written!

34. 34 correct is the logic sane? does it work? bug

    suspects? -> ask!

35. 35 efficient any performance concerns? -> ask!

36. 36 tested unit tests included? unit test opportunities?

37. 37 good practices Code Complete Effective Java Sonar/Findbugs/… codereview.stackexchange.com

38. 38 how bad can it be?

39. 39 how in-depth? •not too much •not too little •just

    right •DO IT FAST

40. 40 attitude?

41. 41 a code review is… NOT about the developer it

    is about the code

42. 42 don’t just say something is “wrong” suggest a better

    way

43. 43 the focus is.. NOT on problems it is on

    solutions

44. 44 perfect code? don’t seek perfect seek good enough better

    is good enough don’t be a pain in the ass be flexible be constructive

45. 45

46. 46 mistakes… it’s OK to make mistakes it’s NOT OK

    to not learn from them

47. 47