DEV Community

Discussion on: The Code Review Checklist ✅

Collapse
 
heroincommunity profile image
German Chyzhov • Edited

Thanks Paul for sharing your experience!

You have a great process, in most companies where I have ever worked we were spending much less time - mostly doing code review in code review tools (Crucible, GitHub etc.), without pulling and running a code in a PR.

Could you please tell, if you think that having the best code reviewers for each PR based on their experience in related code components(parts, modules) is important?

In any case, could you please share your strategy for selecting code reviewers?

Collapse
 
prowe214 profile image
Paul Cullen Rowe

In a large-application, I think it’s definitely valuable to have subject matter experts review the code that’s being developed. The ideal scenario (read: not realistic but what we should strive to do) is that anyone who may touch this area of the application gets eyes on it, understands it, and approves of its implementation.

That said, selecting code reviewers is largely based on your team structure and application organization. You don’t need back end teams reviewing front end code for example. But if there’s someone who even might touch this feature later, they should probably review it, even briefly.

In all of my projects, we enforce a minimum number of approvals before a PR can be merged (usually 2-3). This way we can be sure a few others have seen, understood, and quality-checked the code.

Sorry for my delay in answering!

Collapse
 
heroincommunity profile image
German Chyzhov

Have you tried to use tools like mention-bot, which suggests best reviewers based on commits history?
Or do you use CODEOWNERS or any other things to help with selection of reviewers?

Thread Thread
 
prowe214 profile image
Paul Cullen Rowe

I have not, those sound interesting! I have always organized my teams in either application specific or feature specific groups. This generally makes picking the reviewers easy — pick those people, plus any technical manager or architect who may be involved in overseeing the whole production across teams.

Thread Thread
 
heroincommunity profile image
German Chyzhov

Yes, feature teams is a one way to solve the problem. However, as a developer, I could say, that sticking the whole team to a fixed set of features often leads to lower retention rate.
If in other case, the developers are not sticked to a sed of features, then there could be a mess with responsibility: "who owns XXX" question start appearing often in chats.
As mention-bot is dead and CODEOWNERS is static, I decided to create my own automatic recommendation tool.
Feel free to contact me in Linkedin if interested.