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

Code Reviews

Code Reviews

Just Do It!

Janos Gyerik

October 14, 2015
Tweet

More Decks by Janos Gyerik

Other Decks in Programming

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

    View full-size slide

  2. 2
    what is it?

    View full-size slide

  3. 3
    commit

    commit

    commit

    review

    commit

    commit

    review

    commit

    accept
    commit

    commit

    commit


    NOT code reviewed code reviewed

    View full-size slide

  4. 5
    UCXGVKOG
    catch early

    catch early
    catch early
    bugs
    bad patterns
    ugliness

    View full-size slide

  5. 7
    peer review
    peer pressure

    View full-size slide

  6. 8
    anything that
    gets reviewed
    gets better

    View full-size slide

  7. 9
    quality
    time savings

    cost savings FACT

    View full-size slide

  8. 10
    correct
    readable
    efficient
    tested
    buggy
    messy
    crappy
    untested
    NOT code reviewed code reviewed

    View full-size slide

  9. 11
    information sharing
    NO MORE

    bottleneck developers

    View full-size slide

  10. 13
    why/when

    NOT do it?

    View full-size slide

  11. 14
    big bang
    development too hard to control
    should be the exception,
    not the norm!
    code reviews 

    are not practical when…

    View full-size slide

  12. 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…

    View full-size slide

  13. 16
    what’s
    required?
    disciplined
    commits

    View full-size slide

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

    View full-size slide

  15. 18
    one feature
    one purpose
    one branch
    feature

    View full-size slide

  16. 19
    short-lived
    max 3 days
    feature

    View full-size slide

  17. 20
    (example good branch)

    View full-size slide

  18. 21
    (example bad branch)

    View full-size slide

  19. 22
    one commit
    one logical
    change

    View full-size slide

  20. 23
    one good commit
    ==
    stable build;
    related changes;
    no garbage; small;
    good comment

    View full-size slide

  21. 25
    (example good commit)

    View full-size slide

  22. 26
    example bad commit with many changes)

    View full-size slide

  23. 27
    (example bad commit with garbage)

    View full-size slide

  24. 28
    how to do it?

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  27. 31
    peer review
    peer pressure

    View full-size slide

  28. 32
    what to review?

    View full-size slide

  29. 33
    readable
    is it clear?
    easy to read?
    easy to understand?
    FACT
    Code is read far more often

    than written!

    View full-size slide

  30. 34
    correct
    is the logic sane?
    does it work?
    bug suspects?
    -> ask!

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  34. 38
    how bad can it be?

    View full-size slide

  35. 39
    how in-depth?
    •not too much
    •not too little
    •just right
    •DO IT FAST

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  40. 46
    mistakes…
    it’s OK to make mistakes
    it’s NOT OK to not learn
    from them

    View full-size slide