• Code reviews

    How-to, do & dont's

    Alessandro Lai / @AlessandroLai

    Facile.it - January 10th 2020, Milano

    Source material

    Formal definition

    Code review is the systematic examination of source code,
    and is intended to find mistake overlooked in software development,
    improving the overall quality of software


    That code works as intended.

    Everyone has an opinion

    Are we harder on other people's code than our own?

    We're harder on code while reading

    The ratio of time spent reading code versus writing is well over 10 to 1.
    We are constantly reading old code as part of the effort to write new code...
    [Therefore,] making it easy to read makes it easier to write.
    Robert C. Martin (Clean Code)

    So, what should we look for
    in a code review?


    Different workflows

    • Gateway reviews
    • Knowledge sharing
    • Early design feedback

    So, again:
    what should we look for
    in a code review?

    It depends

    Code review anti-patterns

    • Nit picking
    • Design changes when code works
    • Inconsistent feedback
    • The Ghost Reviewer
    • Ping pong reviews

    Developers hate code reviews

    Code reviews are
    a massive waste of time (?)

    Why (do you do reviews)?

    • Ensure code meets standards
    • Find bugs
    • Share knowledge
    • Check code is understandable
    • Ensure code does what it’s supposed to
    • Collaborate on design
    • Evolve (refactor) application code

    When (do you review)?

    • During implementation?
    • When it's ready to merge?
    • After it's been merged?

    When (the review is complete)?

    • When everyone agrees?
    • When a gatekeeper agrees?
    • When all comments are addressed?


    • Who reviews the code?
    • Who signs it off?


    • Pairing
    • Showing code to a colleague at a computer
    • Mob reviewing
    • Remote screen sharing
    • In the IDE, checking out a commit or branch
    • Using code review software


    Requires you know already:

    1. Why
    2. When
    3. Who
    4. Where

    What to look for (1/2)

    • Fit with the overall architecture
    • SOLID, DDD, Design Patterns or other paradigms
    • New code follows team’s current practices
    • Code is in the right place
    • Code reuse
    • Over-engineering
    • Readable code and tests
    • Testing the right things

    What to look for (2/2)

    • Exception error messages
    • Subtle bugs
    • Security
    • Performance
    • Documentation and/or help files been updated
    • Spelling, punctuation & grammar on user messages



    Human reviewers should be doing what cannot be automated

    Understand the constraints

    • Why: Knowledge Sharing
      Purpose isn’t to reject the code
    • Why: Before merging
      Focus is on how easy is to understand the code
    • When: at the end
      Too late for design, but be specific

    Submitting for review

    • Annotate your code
    • Reviews should be small
      Commits should be smaller and consistent


    • Should be clear Who is reviewing
    • Respond in a timely fashion
    • Checklist of What to look for


    • Bear in mind Why, When and What
    • Be constructive
    • Be specific

    Accept or Reject

    • Accept or request changes
    • Next steps should be clear

    Making changes

    • Respond in a timely fashion
    • Respond to comments (or not?)


    • The goal is to accept the review (and merge?)
    • Should be clear Who signs it off
    • ...and When

    Have clear ojectives

    Clarity comes from understanding

    • Why
    • When
    • Who
    • Where
    • What
    • How

    The Goal Is To Ship The Code

    Not to prove how clever you are