Code review patterns for multiple teams working on a single product

Question

We're scaling out development on a single product from a single team to multiple teams. What are the patterns to ensure coding style, patterns and technology is used consistently?

Is there one person across everything who does nothing but reviews... should more people be involved in reviews? Should reviews take place earlier?

Our currently scrum team size is 6 people with 2-3 developers and 1-2 testers. Pull requests require approval of one developer and one tester... which generally occur after system/integration testing.

I've searched on the internet but couldn't find any guidance.

Answer 1

What are the patterns to ensure coding style, patterns and technology is used consistently?

Automation. For most popular languages there are excellent auto-formatters and linters to ensure a consistent and idiomatic style. Linters should run for every lintable file on every CI run.

Anything important in this category which can't be checked automatically should be documented. I've found that a simple merge request checklist of verifiable statements to be quite useful to collect the way we do things right now. It should consist of easily verifiable statements about how things should be done (not how they can be done; it's project documentation, not a programming guide), ideally with a rationale and examples. That way, when you inevitably end up discovering a better way of doing things you can simply update this document. As a bonus, if the document is part of the repository the change will be reviewed by another team member, so it's verified as being a good idea and readable. An example pulled out of thin air could be something like this:

• [ ] Permissions classes inherit from our custom BasePermission class. Rationale: the Frobnicator library BasePermission class is unsafe because blah blah, but they refuse to fix it because it will break too many things [link to issue]. Example: from dwim.permissions import BasePermission …

Is there one person across everything who does nothing but reviews... should more people be involved in reviews?

You absolutely should make sure all the developers involved in the project do code reviews, including the juniors:

• Domain knowledge is at least as important as "raw" programming knowledge for code reviews, especially once you enforce linting to avoid all those bikeshedding review comments. Someone only reading the code but not implementing changes themselves would get a very lopsided view of the domain, so their reviews would not be as useful as they could be.
• Someone whose only job is reviews would probably quit ASAP because it would be career suicide. There are no jobs as a code reviewer.
• Juniors can be great at spotting overly complex solutions, because if allowed they are likely to ask "why".
• Involving juniors also trains them to do reviews, and to feel like they are part of the team in a different way than just implementing things and receiving feedback.

Should reviews take place earlier?

Earlier than what? Reviews should take place as soon as the author believes the change is ready to merge. Any earlier and you'd be massively increasing risk and waste (in the Agile sense of unreachable code), any later and the author will have lost context by the time the review is finished.