How to survive

code Reviews

Positive aspects

  • Get help to improve your code in matter of performance and design.
  • Improve the readibility of the code.
  • Knowledge sharing.
  • Early Design Feedback.
  • It is a great learning opportunity.

Negative impact

  • Delay deploys.
  • Slow the code evolution.
  • Fall into Anti-patterns.

How we do it (I)

Are we harder on other's people code?

YES!

But are we harder than we would be on
our code 6 months after writing it?

Maybe not?

How we do it (II)

Writing and reading code requires different mindsets:

  • When we write code we are looking to achieve and objective.
  • When we read code the work is done, we are just looking for ways to improve it. Plus, we don't need to do the work, so the sky is the limit.

What you should be looking for?

  • Depends on your team/company
    Code Review policies and processes.
  • If it is not written, you could assume
    from context and share your view.
  • It is really important to know/agree on this
    to avoid frustration and overwork.


Note: In Confusion in Code Reviews paper you can read more
about the negative impact of not knowing.

Traditional getaway code review

  • Small amount of code.
  • Add information for the code reviewer (why you did things the way you did)
  • Respond in a timely fashion.
  • Checklist of what to look for (I'm not looking at the code until I find an error)
  • Write useful comments: Relevant, Actionables, Specific & Constructive (you can use the SMART criteria)
  • Accept or Raise concerns that need to be solved.

Cuentos desde las trincheras

Integrantes:

  • Matias Miranda
  • Diego Sabolo
  • Manuel Santibañez


AKA: Yo era otro infeliz...
(Les Luthiers: El Sendero de Warren Sánchez)

Confesionario

Yo también sufrí los Anti-patterns

Anti-patterns

Nitpicking

Formatting, obvious security mistakes, method size, add unit test, etc...

Best to use automated tools to detect and if something leaks through only comment on it. Drupal wise we have a Coding standards which ideally we should follow.

Anti-patterns

Design changes when the code works

This needs to be attacked earlier, not at the time of delivery. If you think this is needed, it becomes a comment and can be done in future releases.

Anti-patterns

Incosistent Feedback

Again this becomes an opinion. Refrain from it to avoid blocking and make it be a comment.

Anti-patterns

The Ghost reviewer

When you "commit" to do a review, but never get around to do it. So the code is in a wait state, and doesn't get delivered.

Anti-patterns

Ping-pong review

Instead of giving me all at once, with each delivery you have a new opinion.

Anti-patterns

More?

Yes, there are many. You can read more about the presented here or look for more at Anti-patterns in Modern Code Review IEEE paper.

Code Review Workflows

  • Early Design Feedback
  • Knowledge sharing
  • Gateway code review

Lets make fun
about it...

The Ghost reviewer

Make Annotations

Nitpicking

Not using a checklist...

Security Checks

You want more?