Tuesday, 18 December 2018

Are Code Reviews Waste?

I cross posted this post on Medium.

"Waste" is NOT a Dirty Word

My recent talk Agile is a Dirty Word generated a lot of interest at the conference at which it debuted and on social media since. More, in fact, than I ever thought I would get for any of my talks. One of the themes I examined in this talk was waste. This came quite early in the piece and one of the points I was making is that (unlike Agile, Kanban and various others) "Waste" is NOT a dirty word. What I mean by this is that whoever the client is (in my experience at least) and whatever stage of Agile maturity they are at, "Waste" is a concept that people understand and want to reduce.

An "Innocent" Value Stream Mapping Slide

I wanted a way at my client to convey how certain activities don't add value to the business and should therefore be considered to be waste. The slide I came up with for the client was designed to illustrate a very simple value stream map and start a conversation over which activities add value and which are waste. I reused the client slide with only a change to the title in Agile is a DirtyWord. Here is the slide as used in Belgium:


I use the word "innocent" in the title of this section because, as is usually the case, nothing on this slide was ever accidental or innocent. When I originally showed this slide to our client (it has transitions that make parts of it more obvious) I knew that "CODE REVIEW" down in the waste part would be controversial. I knew that several people at our client were obsessed with the idea that code review was an important part of their delivery process. So whilst "code review is waste" was not the title of this slide when I showed it at the client, I very definitely wanted somebody to notice it was flagged as waste and challenge it. This did indeed happen which "coincidentally" let me introduce the subject of why we think code review is waste in the course of talking about something completely different.

What is Waste?

In the rather simplistic model above, I have a notional timeline for a story moving across a wall. We have to assume that the thing that is moving across the wall is valuable. That is to say, I'm assuming that some sensible work by a product owner or similar has taken place and we are confident that when this thing gets released in to production that it will return some value to our company. Note the key point here is that when this thing gets released to production it will return some value to our company - NOT BEFORE. So the clock starts ticking from the point when the story's value statement is articulated and ends when it is live in production. During that time, anything that doesn't help the story move along the wall is waste.

What Outcome is Supported by a Code Review?

One question that we often ask our clients is "what outcome does this process support?" Sometimes you can ask this question of a particularly silly piece of risk management theatre and it can provoke anything from mystification to ultra defensive cat-bear-trap response to complete silence. All three responses, by the way, are a great sign that the process being questioned is unconnected to any real outcome and thus devoid of any value. When it comes to asking this question about code review, however, I've generally received a response along the lines of "code review ensures good code quality" or "we need to check that the code works". So assuming such a response is offered, we can't argue the thing away on the grounds that it can't be linked to a desirable outcome, since "working code" or "code quality" are both outcomes we should be looking for. So why do I argue code review is waste?

What REALLY Happens in a Code Review?

Disclaimer: I am writing this section based on my own personal experience. If anybody has worked in a situation where better things happen with respect to code review I'd love to hear how those better things can be made to happen.

I would also add that all of my comments refer to an organisation where the team is co-located or at most distributed over two sites. I am not considering an entirely distributed team.

The Process

Usually, a code review is requested by a developer when they want to merge some changes into a mainline. This results immediately in the request for code review waiting in a notional queue somewhere for somebody to act upon it. This is undeniably waste, all queuing time is waste, I don't think there is any argument on this one. When somebody "picks up" the code review all they can typically see is the changes made to the code. Depending on the diligence of the reviewer or, more likely, whatever time they perceive they can spare from their "real work", they might fire up their code editor and view the code changes in the wider context of whatever code base it lives. They then annotate the changes (using whatever tool they use) with any suggestions for improvement and update the status of the code review to something like "suggestions for changes made" or whatever the workflow tool in play dictates. At that point the review goes back on the queue of the person who requested the review (more undeniable waste), they react to the suggestions as they see fit and then the cycle repeats until the code reviewer "approves" the changes.

The Comments

Again, I repeat my disclaimer. There are two types of comments that I have seen. The first type are utterly unrelated to anything the code does at all. An example of such a comment would be "We should put opening braces on the same line as a function signature, NOT the line below", or one of my particular favourites "You have used a tab character of width 3, instead of 3 spaces, for your indent" (how did this person even spot that?) These type of comments are not only entirely devoid of value whatsoever, they are irritating, probably passive aggressive (I'm still never quite sure exactly what PA means but I think it applies here) and DEFINITELY should be caught with whatever linting tool is play.

The second type of comment I have seen would be along the lines of "you used a for loop to iterate when you could have used foreach" or "this slightly different way of doing it will be more efficient". Whilst I have slightly more sympathy with this type of comment I still don't think it is worthy of note unless there is specifically something about the implementation that is "wrong" or would be likely to cause some kind of real performance issue.

A Useful Comment

A useful comment could be something like "the tests don't cover all the edge cases" or "There should be an integration test of this thing". I have never seen this happen. Why? I think the answer is that the reviewer lacks the context of the story that was played and therefore lacks the knowledge to make any really useful comments about actual functionality.

Does Code Review Support the Suggested Outcomes?

If we assume that the types of comments that get passed around fall under the not useful categories mentioned above then I struggle to accept that code reviews do anything to support the outcomes of "good code" or "working code" and I would therefore argue that all such process is waste.

What If the Comments are Useful?

This is the tricky part of my argument. I have had some people argue that there are useful comments in the code reviews that happen at their company. As I say, I have never seen this in any review that I've been involved in but I'm prepared to accept that it can (and probably does) happen that some useful comments are sometimes added to code reviews and acted upon by the original developer. Is such a code review waste?

I believe it is still waste. Why? Firstly, the queuing time is there, whatever you do. Secondly, there are at least two context switches involved for different people to support this process. Context switches are expensive and add no value to any process. So they are waste. Thirdly, there is a better solution to the outcome of "good code" and "working code". Both of these outcomes are more easily achieved through pairing or mob programming and you get shared context and therefore better bus factor, into the bargain. Perhaps, if the outcomes actually are being legitimately supported by a code review process then I should downgrade my claim from "code review is waste" to "code review is an anti-pattern". I think that may be semantically more accurate in this case but "waste" is somehow more tangible a thing to eradicate than "anti-pattern".

Can Code Reviews be Value Adding?

I mentioned above that certain code reviews might be arguably an anti-pattern rather than straightforward waste. I'm prepared to accept that they add value if the following things are true:
  • Instead of the workflow, with all the queues mentioned above, the developer requesting the review approaches other developers until she finds somebody available to help with the review.
  • The code review is carried out together between the developer and the reviewer. All suggestions are therefore discussed and collaboratively acted upon.
  • Assuming there isn't the need to do any major extra work (perhaps after sharing the context it becomes apparent that the suggested solution just doesn't do what it is meant to do) the pair will continue until they are satisfied with the result and they will then merge the changes as part of the workflow.
According to the authors of Design Patterns, an anti-pattern has the following characteristics:
  • A commonly used process, structure, or pattern of action that despite initially appearing to be an appropriate and effective response to a problem, has more bad consequences than good ones.
  • Another solution exists that is documented, repeatable, and proven to be effective.
I think I have argued that in general code review fails in its goals and thus fulfills the first part of the definition. I strongly believe that there exists at least two better solutions - pair programming and mob programming. So it is therefore my contention that code review is at best an anti-pattern and at worst (and this is the more likely scenario in my experience) waste.


42 comments:

  1. there are portable wine coolers which also fit in a small office space. i use them in my home office** ReviewsByPeople

    ReplyDelete
  2. Thanks for posting this info. I just want to let you know that I just check out your site and I find it very interesting and informative. I can't wait to read lots of your posts. Junk Car Removal

    ReplyDelete
  3. Hi there, I found your blog via Google while searching for such kinda informative post and your post looks very interesting for me. JUNK REMOVAL SERVICE

    ReplyDelete
  4. I think that I'm into your pieces of writing. I wait for your posts every week. I learn many new interesting things from the articles. The information is essential for me. I want to thank you for sharinghanks for the website article.Really thank you! Significantly obliged.
    junk trash removal austin

    ReplyDelete
  5. bachelor developer software company
    hello brother's your bachelor developer software company provide high qualty software provider ricently create a school menegment system provide a very low bujet for school's with a androide application.

    SEO
    hello brother's your software company provide a high quality seo websites ranking on page and off page.

    graphic design
    hello brother's our software company provide graphic designing service by bachelor developer.

    software company in hyderabad
    software company in pakistan

    hello brother's your bachelor developer software company provide high qualty software provider ricently create a school menegment system provide a very low bujet for school's with a androide application.
    Web design company

    bachelor developer software company provide high quality software and websites and more services.
    cheap website design pakistan

    software house in pakistan

    software house in sindh

    web development company pakistan

    software house in latifabad

    ReplyDelete
  6. OFF PAGE
    I create a cheap rete off page seo WOW, WHAT A EXCELLENT POST. I REALLY FOUND THIS TO MUCH INFORMATICS. IT IS WHAT I WAS SEARCHING FOR.I WOULD LIKE TO SUGGEST YOU THAT PLEASE KEEP SHARING SUCH TYPE OF INFO.THANKS
    SEO LINKBUILDING
    I I create a cheap rete seo link building reading some of your content on this website and I conceive this internet site is really informative ! Keep on putting up.
    BACKLINKS
    I create a cheap rete backlinks seo Megan this is a great way to organize them all!! I have tried to hand write them, but I always forget when they are or what I have linked up. Thank you very much for sharing this with us at TaDa Thursday!!
    NICHE COMMENTS
    here is my great offer i will provide you 80 niche relevant hight quality blog comments with high authorize sites good for your website rank your site on google, bing, yahoo etc. All comments are manually No software use.
    SOCIAL BOOKMARKING
    Great job for publishing such a beneficial web site. Your web log isn’t only useful but it is additionally really creative too. 
    WEB2.0
    here is my great offer i will provide you 80 niche relevant hight quality blog comments with high authorize sites good for your website rank your site on google, bing, yahoo etc. All comments are manually No software use.
    WEB2.0 PROFILES
    xtremely pleasant article, I appreciated perusing your post, exceptionally decent share, I need to twit this to my adherents. Much appreciated!.
    SOCIAL MEDIA SERVICE
    We are truly thankful for your blog entry. You will discover a great deal of methodologies in the wake of going to your post. I was precisely scanning for. A debt of gratitude is in order for such post and please keep it up. 
    BLOG COMMENTING SERVICE
    I was reading some of your content on this website and I conceive this internet site is really informative ! Keep on putting up.
    BLOG COMMENTING PACKAGES
    Nice post! This is a very nice blog that I will definitively come back to more times this year! Thanks for informative post.
    HIGH AUTHORITY BACKLINKS
    I am getting this error message in the Promoted Link Tiles itself. In the left navigation its working though. Also I am able to verify the address while adding it in the promoted links.
    HIGH AUTHORITY BACKLINKS

    ReplyDelete
  7. I’m curious to find out what blog system you’re utilizing? I’m having some small security problems with my latest website and I’d like to find something more safe. Do you have any suggestions? And Bet Numbers

    ReplyDelete
  8. Your blog provided us with valuable information to work with. Each & every tips of your post are awesome. Thanks a lot for sharing. Keep blogging see this page.

    ReplyDelete
  9. This is a great inspiring article.I am pretty much pleased with your good work.You put really very helpful information. Thank you a lot! and if you need Graphic Design Company then contact us!

    ReplyDelete
  10. On the off chance that the offer is on crude sustenance, at that point you should reconsider in light of the fact that you may need to process them suitably before you can store them. Another choice is to get someone else to pool buys with you so you don't need to manage a capacity crisis. coupon code

    ReplyDelete
  11. This is an awesome post.Really very informative and creative contents.
    Logo design company in chennai

    ReplyDelete
  12. These are really amazing and valuable websites you have shared with us. Thanks for the informative post.
    miami dade county services

    ReplyDelete
  13. I’m going to read this. I’ll be sure to come back. thanks for sharing. and also This article gives the light in which we can observe the reality. this is very nice one and gives indepth information. thanks for this nice article... buy google 5 star reviews

    ReplyDelete
  14. Thanks for sharing this information. I really like your blog post very much. You have really shared a informative and interesting blog post with people. soil testing new jersey

    ReplyDelete
  15. Our easy-to-use website and the multitude of custom stickers printing option are just some of the reasons why business, graphic designers, and all our customers trust us with their printing – all backed by our 100% Guarantee!
    Eyyaa

    ReplyDelete
  16. Our easy-to-use website and the multitude of custom stickers printing option are just some of the reasons why business, graphic designers, and all our customers trust us with their printing – all backed by our 100% Guarantee!
    Ey Yaa

    ReplyDelete
  17. I am glad to read this post, it's a good one. I am always looking for quality posts and articles and this is what I found here, if anyone looking to softwarte devlopement service, then visit custom software development company

    ReplyDelete
  18. code as a starting point for your project ... it definitely handles multiple clients. Regarding the message about the 'Task' class ... the 'Task' class is found in the productrapid .Net 4.0 assemblies and should be already referenced from a .Net 4.0 console ap

    ReplyDelete
  19. If you visit other cities and you want to have awesome fun then please meet my friends who are hot and sexy meet these wild beauties for the pleasure you are seeking I assure you that these young women are incredibly sexy and passionate to the degree that you imagine your wildest fantasies will come true hurry up and call them you can know a little about them on this page and then you can choose which one to meet you must grab this opportunity of being with a fine woman with delicate features and romance them so that your life is cherished forever you can go to these towns to have these beauties Watch Series with you and enjoy your life to the fullest just go and book a hotel and then invite these girls to your room for some adventure.

    ReplyDelete
  20. Free Web Directory is an online catalog of websites arrange by categories. Highly specialized SEO friendly human edited free web directory. Submit your websites free with deeplinks. Free Web Directory

    ReplyDelete
  21. Links Catalog – Premium Link Directory Features high quality and well-designed websites curated by human editors and organized by category. Website Premium Link Directory listings for businesses, companies, products, and services organized in relevant categories. Links Catalog increase your domain authority for a higher rank on search engines.

    ReplyDelete
  22. Your blog provided us with valuable information to work with. Each & every tips of your post are awesome. Thanks a lot for sharing. Keep blogging.

    telemedicine app development

    ReplyDelete
  23. I think this is an informative post and it is very useful and knowledgeable. therefore, I would like to thank you for the efforts you have made in writing this article.online marketing firm

    ReplyDelete
  24. Thanks for posting this info. I just want to let you know that I just check out your site and I find it very interesting and informative.
    austin junk removal companies

    ReplyDelete
  25. Hello I am so delighted I located your blog, I really located you by mistake, while I was watching on google for something else, Anyways I am here now and could just like to say thank for a tremendous post and a all round entertaining website. Please do keep up the great work. logo packages

    ReplyDelete
  26. Wow! Such an amazing and helpful post this is. I really really love it. It's so good and so awesome. I am just amazed. I hope that you continue to do your work like this in the future also seo-ranking

    ReplyDelete