Code review comments at different levels

Most of the developers are introverted: most likely they prefer interacting with their computers, rather than interacting with humans. Understanding how computers work is pretty straightforward. Once a code is written, it will be executed the same way one after the other. Computers are predictable. Unlike humans.

Whenever a team is created, developers need to communicate with each other, which requires proper level of EQ and soft skills from all team members, otherwise they would be in storming phase for quite a long time. Human-human interactions need to be polished, and conflicts need to be managed around the team.

For a software developer, code review is one of the most important communication channels. Pull request reviews can also be considered as danger zone, or a potential area where conflicts can arise. Most of the times, codebase is being considered as an output of an “art” process. Whoever wrote the code is proud of his/her code. For him/her, the logic of the implementation is straightforward and clean. The owner might think that there is no better way to implement the feature. Reviewers might have different point of views, and different standards, however. This is when values and standards collide, making huge conflicts over a couple of lines of code. The truth is that a feature can be implemented in thousands of correct ways, and millions of wrong ways. Reviewers need to mitigate which solution should be picked, out of the “thousands of correct ways”.

Before jumping into the different levels of PR comments, lets see an interesting model from Prof. Werner Gritt — Information Theory. Werner defined the following abstractions of meaning: statistics, syntax, semantics, pragmatic, apobethic.

Consider the following sentence: “If you keep asking me, I will kick you in the butt!”

• Statistics: at statistics level, the sentence is just a set of characters. You can count 1 “a” character, 1 “b” character, 4 “e” characters, and so on.
• Syntax: at syntax level, we see that the characters are grouped into words, and these words are ordered and compliant to linguistic rules.
• Semantics: at semantics level, we see that this is a sentance, which has a meaning.
• Pragmatic: at pragmatic level, we can interpret the message “literally”. You can envision a person who is asking too much, then someone else is literally kicking the guy’s butt with his boots.
• Apobethic: at apobethic level, the message is something like this: “Please stop asking me, because I am getting frustrated”. Note that at this abstraction level, we extract the “underlying message”, which is basically an intention.

Now lets see how it translates to Pull Request comments

• Statistics: at statistics level, a code review comment would be something like this: “you have added 800 new lines, but you only deleted 23 lines. this pr can not be merged, you are increasing the complexity of the codebase”. Complaining about the number of lines is easy, and it looks reasonable as well. It does not require the reviewer to even take a look into the content of the Pull Request, it is completely enough to just read the summary. These comments bring low value to the review, and are most likely to be present just for the sake of visibility.
• Syntax: based on my experiences, more than 80% of the comments are made at this level. Examples: “you could have done that i += 1 by just using i++”. Or another one could be “that curly bracket should be placed to a new line”. As you can see, these comments are not adding any value to the review either, these are mostly declaring “preferred” ways from the reviewer. Who cares if Alice likes “i++” form, but Bob wrote “i += 1”. In the end, a variable will be incremented by 1 anyways. Both ways are easy to understand for all developers. Developers can keep fighting over these for so long. Best way to avoid this is to introduce a super sensitive static code analyzer, that runs in pre-commit hook.
• Semantics: when a developer is reviewing the codebase at this level, he/she would not only see tons of characters and ternary operators, but he/she would also execute the modified codebase in his/her mind. The changelist has an effect to the behaviour of the codebase. Example: “I see that you wanted to use Quicksort here, but your implementation is not quite right, because you are never touching the item at 0 index”. This is the first level by the way, where brain is being used.
• Pragmatic: at pragmatic level, we go even further. A reviewer might open the linked JIRA ticket, check out the feature branch, then compile the codebase, and test the implementation against the acceptance criteria. I have only seen very few developers who have done this. Comments could be like “The specification says that we need to keep all data in cache, but you forgot to add @Cached annotation to this repository. Is there any reason?”. Personally I really like these comments, I find them valuable.
• Apobethic: at this level, the reviewer would not only check the modifications against the specification (eg. acceptance criteria of the JIRA ticket), but he/she would think over the whole process, and try to find gaps. Comments can be like this: “It is nice that you fetch the data from database, but did you consider that the application is running on distributed environment? The data that you are trying to fetch might not be available due to a delay in replication. I think your code should be prepared for that scenario as well.”. Comments like this are extremely powerful. These comments can prevent outages and endless bug investigations.