Non-Blocking Continuous Code Reviews - a Case Study

Slide 1

Slide 1 text

in/tdpauw @tdpauw.bsky.social @tdpauw@mastodon.social thinkinglabs.io Non-Blocking Continuous Code Reviews Thoughtful Thierry 2021 Ghent lightening festival Kunsthal Gent Lange Steenstraat

Slide 2

Slide 2 text

Why are we doing Code Reviews? 🤔

Slide 3

Slide 3 text

But few teams have made a conscious decision about what they want to achieve with their code reviews. – Pia Fåk Sunnanbo, I Hate Pull Requests

Slide 4

Slide 4 text

My reasons … ● Mentoring, learning ● Knowledge sharing ● Check on readability and maintainability ● Maintain consistency in design My reasons … ● Mentoring, learning ● Knowledge sharing ● Check on readability and maintainability ● Maintain consistency in design (that was a mistake, should have been a design workshop)

Slide 5

Slide 5 text

Other reasons …

Slide 6

Slide 6 text

4-eyes principle Killer argument to slow everything down and drive down quality. Pair or Team Programming are more eﬀective

Slide 7

Slide 7 text

“Sense” of security. I don’t trust my code. Dilution of responsibility. -> won’t be ﬁxed by a process

Slide 8

Slide 8 text

What Code Reviews should not be?

Slide 9

Slide 9 text

● By seniors missing mentoring skills ● Blaming or harming ● A miserable experience ● Gatekeeping ● A must ● Exercise power

Slide 10

Slide 10 text

Non-Blocking Continuous Code Reviews

Slide 11

Slide 11 text

A Team’s Tale

Slide 12

Slide 12 text

Two key ideas … Establish a continuous review process.

Slide 13

Slide 13 text

No content

Slide 14

Slide 14 text

No content

Slide 15

Slide 15 text

If we commit changes directly to the remote mainline, we have unreviewed code alongside reviewed code.

Slide 16

Slide 16 text

Second key idea … Stop the code review being deﬁnitive for release.

Slide 17

Slide 17 text

Unreviewed code could end up in production, whether the feature was ﬁnished or not.

Slide 18

Slide 18 text

Unreviewed code ≠ a bug !! => automated and exploratory tests catch bugs

Slide 19

Slide 19 text

The unreviewed code is code that works. It passed all tests!

Slide 20

Slide 20 text

Beneﬁts

Slide 21

Slide 21 text

Transaction Cost the cost of sending a batch to the next stage

Slide 22

Slide 22 text

thinkinglabs.io @tdpauw@mastodon.social @tdpauw.bsky.social It it takes 10 min to make a change, the change is blocked until it is reviewed and it takes 2 hours to get a review … 92.3% wait time 😳 High cost of code review! => ask less often for a review 🤷

Slide 23

Slide 23 text

Expensive process works against making small changes -> less refactoring -> less incremental development

Slide 24

Slide 24 text

the wait time incentivises Work in Progress

Slide 25

Slide 25 text

Little’s Law the higher the WIP the more delay

Slide 26

Slide 26 text

To get something out sooner … need to reduce the transaction cost! Minimise the cost of code review.

Slide 27

Slide 27 text

Beneﬁts (bis)

Slide 28

Slide 28 text

Transaction Cost is nearly zero. => work in small increments

Slide 29

Slide 29 text

With Non-Blocking Code Reviews we never slow down, changes are never blocked for delivery. No gating!

Slide 30

Slide 30 text

With Non-Blocking Reviews we have early, fast feedback.

Slide 31

Slide 31 text

With Non-Blocking Code Reviews no new work is started before ﬁnishing. => less Work in Progress => because Little’s Law: less delays

Slide 32

Slide 32 text

With Non-Blocking Code Reviews no pressure to improve code quality. Nobody is waiting.

Slide 33

Slide 33 text

With Non-Blocking Code Reviews there is high trust.

Slide 34

Slide 34 text

Tooling

Slide 35

Slide 35 text

● We used: Atlassian Crucible (discontinued)

Slide 36

Slide 36 text

● JetBrains UpSource (discontinued) ● Facebook Fabricator (discontinued)

Slide 37

Slide 37 text

● Gerrit (review Git repo in front of main Git repo)

Slide 38

Slide 38 text

● Auctionet Ex-remit (barsoom/ex-remit on GitHub) ● ReviewBoard (similar to Crucible)

Slide 39

Slide 39 text

● Or … no tooling at all 🤷

Slide 40

Slide 40 text

Gotcha with Non-Blocking Code Reviews

Slide 41

Slide 41 text

It does not work for regulated industries. => Pair or Team Programming is the better solution

Slide 42

Slide 42 text

Conclusion

Slide 43

Slide 43 text

Pair or Team Programming are still a superior delivery option. But can be a cultural stretch.

Slide 44

Slide 44 text

thinkinglabs.io Non-Blocking Review strategies are signiﬁcantly better than any of the alternatives. ● No gating ● Deliver faster without delays ● Less WIP ● No context switching ● No urgency to ﬁx quality issues ● Features grow incrementally, commit by commit ● Encourages refactoring

Slide 45

Slide 45 text

in/tdpauw @tdpauw.bsky.social @tdpauw@mastodon.social thinkinglabs.io Acknowledgments: Stefan De Moerloose (https://linkedin.com/in/stefandemoerloose/) for having been that amazing team manager open to try out all our crazy ideas. Dave Farley (@davefarley77) for nudging me to write this down. Giovanni Asproni (@gasproni@mastodon.social) for poking me to turn this into a positive story. Lanette Creamer, Wouter Lagerwije, Diane Gombart, Falk Kühnel and Steve Berczuk for reviewing the slide deck. Laphroaig and Bunnahabhain for the creative support. The Article: https://thinkinglabs.io/articles/2023/05/02/non-blocking-continuous-code-reviews-a-case-study.html Hello, I am Thierry de Pauw

Slide 46

Slide 46 text

Resources Optimizing the Software development process for continuous integration and ﬂow of work, Martin Mortensen The Article: Non-Blocking Continuous Code Reviews, a Case Study, Thierry de Pauw Code Complete, Steve McConnell Facts and Fallacies of Software Engineering, Robert Glass Joel on Software, Joel Spolsky What is the purpose of Code Reviews, Thierry de Pauw, a discussion on LinkedIn I’ve found something better than PRs, the video from Dave Farley on the topic From Async Code Reviews to Co-Creation Patterns, Dragan Stepanović I Hate Pull Requests, Pia Fåk Sunnanbo Problems with Pull Requests and How to Fix Them, Gregory Szorc On the Evilness of Feature Branching - But Compliance, Thierry de Pauw How to make sure tests are of quality, Thierry de Pauw, a discussion on LinkedIn