which of those are the most valuable issues to look for?<\/strong> <\/p>\n\n\n\nWhich code review feedback is the most valuable?<\/h2>\n\n\n\n Let\u2019s pack up a bit and let us imagine again how you\nactually start the code review. <\/p>\n\n\n\n
Probably once you open the review, you will look through all the files and start orientating yourself. Where and what has been changed? What parts of the code are affected? Where is the center of the change? <\/p>\n\n\n\n
And while you familiarize yourself with the change, you will notice some issues: typos in comments and variables, missing comments, style-related issues such as indentation. And even though these are relevant and beneficial issues to look for, don\u2019t get caught up in those issues. The fact is, those are valuable and relevant but shouldn\u2019t be the main purpose of code reviews. I’ll explain in a bit. <\/p>\n\n\n\n
So, what else are you looking for? <\/p>\n\n\n\n
Feedback on defects, missing validation, and best practices are most valuable<\/h2>\n\n\n\n Yes, the most valuable code review feedback is about actual problems with the code. This type of feedback is perceived by all developers as the most useful and beneficial. But, in the study, we also saw that those issues make up only a small part of all the code review feedback. <\/p>\n\n\n\n
In the figure below, you see which kind of issues are discussed in code reviews, and also what their perceived value is from the perspective of the code review author. So what do we see?<\/p>\n\n\n\nTypes of code review feedback and its usefulness<\/figcaption><\/figure>\n\n\n\nWe see that the most valuable comments in code reviews\naddress the following issues:<\/p>\n\n\n\n
Functional defects.<\/strong> This is like a no-brainer. The most highly rated code review feedback is when a reviewer finds a functional defect in the system. But, code reviews are not the best tool to find functional defects. In fact, only a very small part of all comments are about functional defects. Nevertheless, if one is found, the pay-off of code reviews is obviously huge.<\/li>Missing validation and corner cases. <\/strong>Code review feedback that shows validation scenarios that have been forgotten, problems with the logic, or corner cases that have not been covered is also highly valued by developers. This feedback revolves around finding edge cases and alternate scenarios in which the current implementation would fail.<\/li>Best practices and coding conventions.<\/strong> Code reviews are very beneficial for ensuring a coherent, maintainable, and understandable codebase. Therefore, feedback that points out and identifies code that does not follow coding conventions or best practices is also very highly valued.<\/li>API usage and design patterns<\/strong>. Other highly valued code review feedback focuses on how APIs or third-party libraries are used correctly and on missing or wrongly implemented design patterns.<\/li><\/ul>\n\n\n\nCode Review feedback can be a two-sided sword<\/h2>\n\n\n\n Some of the issues discussed in code reviews do not reveal their value as easily as finding defects. Depending on the concrete feedback and circumstances those comments can either be valuable or wasting everybody\u2019s time. Maybe you have already guessed it. But we are talking about issues with the coding style, coding conventions, and comments. Often such issues are referred to as nit-picking issues.<\/p>\n\n\n\n
Documentation, coding style, and coding conventions.<\/strong> Addressing missing or outdated documentation, highlighting typos in comments, or pointing to badly named variables, is the feedback you will get often during code reviews. But is this really valuable?<\/p>\n\n\n\nSometimes the value of such feedback isn’t immediately visible to the code reviewers. And finding a typo isn’t really a biggy, is it? Well, the real value of such comments comes from the compound effect over time. Resolving such issues fast ensures the code base stays comprehensible and maintainable over time. <\/p>\n\n\n\n
Still, they can come across as “nit-picking”, and already the word has a negative connotation to it. So, teams should make sure the value of such comments is understood by the whole team. <\/p>\n\n\n\n
On the other hand, it’s important to avoid lengthy and repeated discussion on such issues as indentation, or coding style. This definitely slows the productivity of teams down. To ensure teams stay productive, let\u2019s settle on one style convention and move on! <\/p>\n\n\n\n
A no-go is feedback that is beyond the code review purpose<\/h2>\n\n\n\n Most feedback that focuses on the current scope of the code review is seen as valuable. But, the scope of the code review isn\u2019t all code that is visible in the files within the code change. Neither is it beyond the purpose of the code change. Because of that, opening up new issues that are out of the scope for the current implementation is experienced as not useful<\/strong> for the majority of the developers.<\/p>\n\n\n\nAlternative implementations.<\/em> Even though alternative solutions that improve the code are seen as valuable, discussions of alternative implementations that have no obvious benefit for the current piece of code aren\u2019t in favor of your team’s productivity.<\/li>Existing technical debt and refactoring<\/em>: Similar, starting to highlight old technical debt or potential refactoring opportunities is beyond the scope of a regular code review. Those issues should be discussed in a separate thread.<\/li>Planning and future work.<\/em> Another not useful feedback type are comments that highlight future work or work that isn\u2019t planned for the current development cycle. If you see such issues, take notes in tools such as backlogs, or issue tracker, where they are actually valuable for your team. And then, discuss them in situations where such discussions are appropriate. <\/li>Asking questions merely to understand the implementation<\/em>. Even though code reviews are an extraordinary tool for learning and to spread knowledge within the team, asking questions merely to understand the code isn\u2019t the purpose of code reviews. Do not forget, the main goal of the code review author is to get the code approved so she or he can move on. <\/li><\/ul>\n\n\n\n<\/div>\n\n\n