The culture of code reviews @ ZEISS

Everyone doing software engineering understands the importance of code reviews, and there is an almost unlimited number of articles that talk about code reviews best practices and how to perform them.

Today I want to share with you a different angle on the topic, discussing how we use code reviews at ZEISS not only to improve the quality of our codebase over time but also to build stronger teams, improve cross-team collaboration and promote learning through mentoring.

A few words on quality

The primary purpose of the code review is to make sure the overall code health and quality of ZEISS’s codebase is improving over time. In order to achieve this, we created a strong change review process, which relies on the senior engineers on the role of code reviewers to own the CL (change list) and balance the trade-offs between the feature sets and the engineering rules on the code.

As a general rule, reviewers should favor approving the CL once the code is in a state where it suffices its purpose, it’s properly documented and improves the overall code health.

Code health or code quality can be relative to each project or team, depending on their specific needs and programming language in use, however, there are some concepts which apply during the code review phase which includes:

• The code is well designed
• The code suffices its purpose
• There are no broken UIs
• The code is easy to follow and understand
• The changes are tested
• The code follows proper naming conventions
• The code is properly commented
• The code follows the style guide
• The code is properly documented

Tooling

Whenever possible we make use of tools that help us validate a number of items before the CR reaches the reviewer. Tools can help us automate the validation of coding style (with tools like eslint), test coverage and test results, naming conventions, etc.

Mentoring

Code reviews are like knowledge exchange sessions, and they can be an important part of teaching developers best practices, or something new about a language, a framework, or general software engineering principles.

During this session, the reviewer will comment over all the findings identified from his/her experience with 3 different types of comments:

1. Necessary changes
2. Suggested changes
3. Informative

Necessary changes

Enforceable changes are changes that are mandatory to resolve in order for the reviewer to approve the code. Though we encourage reviewers to be flexible with personal preferences, we do follow the rule “Technical facts and data overrule opinions and personal preferences”.

Enforceable changes are those which guarantee our code health and quality.

Suggested changes

These are comments that are placed by the reviewer to suggest an alternative solution for a given problem or suggestions for a different way to accomplish the same. If there are no big reasons to rewrite the code, the reviewee can opt to do the changes or to keep the code as-is.

Informative

These are comments which do not require resolution, they are of informative character, and they are used just to provide additional information among the parts. A good example could be a summary of the reviewer’s thoughts over the CL. They provide a path for more generic feedback which can help to build the relationship between the parts.

In any case, all comments are intended to be constructive feedback, and a way to transfer knowledge between the reviewer and the reviewee, or vice-versa. There are some rules any comment should follow to be effective for this purpose:

• Be kind
• Explain your reasoning
• Enable the reviewee to solve the problem by just pointing out the issues with the code rather than giving an explicit solution
• If the code is hard to follow, kindly ask to simplify the code rather than explaining the code to you

Building stronger teams

At ZEISS, we have a strong team culture. Teams are empowered, autonomous and fully functional units. To maintain them productive we make sure there is respect and trust among team members, but also, that they have fun while working on the projects.

Code reviews as discussed play a crucial role in supporting the individual developer’s growth and learning, but it also helps build better relationships and trust. And this is true not only for my own team members but also for people among different teams in the organization.

As we covered before in our microservices / microfrontends architecture, each team project is connected to the overall Digital Customer Companion (portal.zeiss.com), and we often use common services or libraries that every team can upgrade. Thus it can be very hard to keep up with the changes to a popular service when it can be altered by multiple teams.

Code reviews help us to keep it under control, and keeping everyone informed of what is occurring. As code reviews that happen in the form of a PR (pull request) are not limited only to members of my own team, reviewers who are interested in a particular service, a library, or a microfrontend can review the code of any team as long as it follows the guidelines.

But it is often the case that the engineer creating the PR will assign it to specific people from their own team, and from other teams that are experts on that particular area or code repository. This creates a balance and accountability cross teams that help increase the code quality and bring teams together.

There’s a lot to talk about how our teams perform to deliver the quality products ZEISS is known for but would be the subject of its own article.

Conclusion

Code reviews are at the heart of our performance and quality, they are not an annoyance in the process but rather an opportunity to learn and build better code.

Fun fact: this blog post has been subjected to a cross-team code“post” review.

Thanks for reading!

© Luca Bravo on Unsplash

About Post Author