The hidden dimensions of code review(s) – part 1

Context

The title image was created last year in December with the help of three friends (Alexandra Vasile, Marius Jantea and Ionuț Albescu) over lunch. Since then, I try to write about this subject. The trigger point was when, again, I saw a collocated team using exclusively a tool for code reviews and no personal discussion. It was clear to me that I was seeing some effects of bad things that happened there before and people were hiding themselves behind tools – is so easy to do this.

When I asked them why they did it in this way, they had a perfect excuse: audit. Smart things can be done so that an audit will not generate stupid behaviors. I saw that in our industry, at least here in my area, this audit is a good excuse for bad behaviors/recipes. I think there is a gross misinterpretation of the audit intention and on what should be done. 

So in this case, as in many others, it was about abnormal/bad work environment, lack of understanding subjects/topics  involving our day to day work(audits, code review(s), testing,… ). 

The good thing is that it forced me to think very seriously on code reviews and what it meant for my friends and I. I have to say that when  discussing with my friends, I had in mind the way how James Bach and Michael Bolton analyses certain things(1). 

Why is so important

I continuously saw how this subject, in my opinion, was badly treated. But why it irritated me so much? Well, among other things, because:

– this is how I learned a lot of programming stuff;

– it is so obvious the connection with testing and how helpful is to find bugs;

-it helped me see the connection with the idea of complexity(those things which are named boundaries and how the system will adapt/shape/modelate based on those) (2) ;

– the serious analysis which is being(should be) done can help us arrive at profound stuff;

Checklists

There was a time in my professional life when it was enough to say the words “checklist” and “tool”, when a problem popped out, and everything would have been OK in the eyes of certain managers. I think that those managers wanted simplicity, but most often we deal with an essential complexity. Somehow I felt that, for those managers, the situation awareness always could have been reduced to some simple/obvious things in a very fast way. Everything was reduced to recipes.

Context is everything. This means situations can have different dynamic. I am afraid that for those managers all the situations were all simple/obvious( in Cynefin terms)(3). So for them it was assuring that having that checklist would have acted as a guard with, almost, ad litteram actions described in that checklist – and it makes sense for the Obvious domain in Cynefin, but we are not always there, not at all. 

What if a checklist in complex domain, or liminal zone between complex and complicated, has a different form/significance? What if it means something else? I think that at this level we speak about compressed knowledge, cognitive activations, heuristics. They can’t be used ad litteram, is a lot of uncertainty. I understood this when I had the Rapid Software Testing course with James Bach. James had a list of  different words regarding testing on a slide, it was a huge slide full with words. Someone could have transformed that list in a checklist but…not ok. Each word from there could have been described in a lot of text and lots of actions dependent on context. 

This is how I realized that a professional expert might use a checklist in a different way than a junior would use it.

Maybe also with checklists, in these kind of contexts, the secret is … checklists “all the way down”(4) – and this is a whole different story, for sure not a simple one. James Bach said it so nicely: “Heuristics are often presented as a checklist of open-ended questions, suggestions, or guidewords. A heuristic checklist is not the same as a checklist of actions such as you might include as “steps to reproduce” in a bug report. It’s purpose is not to control your actions, but help you consider more possibilities and interesting aspects of the problem.”(5)

So, to come back to checklists as we know them which are used, most often as I saw it, for process compliance, just tick in the box – which for certain people, not professional who play good politics, can use to cover their ass :(. 

Pull requests

Instead of hearing the words “code review”, I often hear the words “pull request”. And of course the link of that “pull request” was sent by chat to the collocated colleagues. I would like to say that this is something new for me, but it is not. But what is even stranger, is those collocated people decide to do exclusively the code review via the tool. This reminds me of the team who was doing daily meetings via Skype although they were in the same office… 

I know there can be good scenarios to make code review via the tool. But to have it exclusively, no matter what, via the tool for collocated colleagues is rather problematic.  

The justification for pull request(s) most often is, again, the audit.

Why dimensions

I noticed that code review has only a narrow perspective in the eyes/actions of many people. Some aspects, intentionally or unintentionally, are not considered, like: the environment, the context of where that code lives, what generated that code, the possible unintended consequences of the code,… 

Is not enough to concentrate on those lines of code only. I say this because of 2 reasons:

– if something is not seen/observed does not mean it does not exist.

– I think that code has a complex aspect in it. This means that we have to see the connections also. Because in complexity is all about connections. 

The title image from this blog post was created to show the possible dimensions worth considering, at a certain moment, when doing a code review. It is a guideline.

Depending on the moment when code review is being done, certain dimensions(see the image below)  can be ignored while others will matter more. For example, in the image below, I want to concentrate on seeing how SOLID(6) principles were applied guided by the business need:

In the part 2 and part 3 of this series we (Ionut Albescu and I) will tackle these dimensions. 


(1) James Bach, Michael Bolton, ”A Transpection Session: Inputs and Expected Results”,  https://www.developsense.com/blog/2010/05/a-transpection-session-inputs-and-expected-results/ 

(2) Dave Snowden, “Boundaries”, https://cognitive-edge.com/blog/boundaries/  

(3) Dave Snowden, “Cynefin Framework Introduction”, https://cognitive-edge.com/videos/cynefin-framework-introduction/ 

(4)”Turtles all the way down – is an expression of the problem of infinite regress” , https://en.m.wikipedia.org/wiki/Turtles_all_the_way_down 

(5) James Bach, “Heuristic Risk-Based Testing”, http://www.testingeducation.org/course_notes/bach_james/cm_2002_rapidsoftwaretesting/rapidsoftwaretesting_13_heuristic_risk_based_testing.pdf 

(6) Robert C. Martin, “The Principles of OOD”, http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod 

Leave a Reply

Your email address will not be published. Required fields are marked *