{"id":318,"date":"2020-08-06T06:00:00","date_gmt":"2020-08-06T06:00:00","guid":{"rendered":"https:\/\/blogs.zeiss.com\/tech\/?p=318"},"modified":"2020-08-25T09:40:23","modified_gmt":"2020-08-25T09:40:23","slug":"the-culture-of-code-reviews-zeiss","status":"publish","type":"post","link":"https:\/\/blogs.zeiss.com\/tech\/the-culture-of-code-reviews-zeiss\/","title":{"rendered":"The culture of code reviews @ ZEISS"},"content":{"rendered":"\n
Everyone doing software engineering\nunderstands the importance of code reviews, and there is an almost unlimited\nnumber of articles that talk about code reviews best practices and how to\nperform them.<\/p>\n\n\n\n
Today I want to share with you a different\nangle on the topic, discussing how we use code reviews at ZEISS not only to\nimprove the quality of our codebase over time but also to build stronger teams,\nimprove cross-team collaboration and promote learning through mentoring.<\/p>\n\n\n\n
A few words on quality<\/h2>\n\n\n\n
The primary purpose of the code review is to\nmake sure the overall code health and quality of ZEISS\u2019s codebase is improving\nover time. In order to achieve this, we created a strong change review process,\nwhich relies on the senior engineers on the role of code reviewers to own the\nCL (change list) and balance the trade-offs between the feature sets and the\nengineering rules on the code.<\/p>\n\n\n\n
As a general\nrule<\/strong>, reviewers should favor approving the CL once the code is in a state\nwhere it suffices its purpose<\/strong>, it\u2019s properly documented<\/strong> and improves the overall code health<\/strong>.<\/p>\n\n\n\n
Code health or code quality can be relative to\neach project or team, depending on their specific needs and programming\nlanguage in use, however, there are some concepts which apply during the code\nreview phase which includes:<\/p>\n\n\n\n
The code is well designed<\/li>
The code suffices its purpose<\/li>
There are no broken UIs<\/li>
The code is easy to follow and\nunderstand<\/li>
The changes are tested<\/li>
The code follows proper naming\nconventions<\/li>
The code is properly commented<\/li>
The code follows the style guide<\/li>
The code is properly documented<\/li><\/ul>\n\n\n\n
Tooling<\/h2>\n\n\n\n
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.<\/p>\n\n\n\n
Mentoring<\/h2>\n\n\n\n
Code reviews are like knowledge exchange\nsessions, and they can be an important part of teaching developers best\npractices, or something new about a language, a framework, or general software\nengineering principles.<\/p>\n\n\n\n
During this session, the reviewer will comment\nover all the findings identified from his\/her experience with 3 different types\nof comments:<\/p>\n\n\n\n
Necessary changes<\/li>
Suggested changes<\/li>
Informative<\/li><\/ol>\n\n\n\n
Necessary changes<\/h3>\n\n\n\n
Enforceable changes are changes that are\nmandatory to resolve in order for the reviewer to approve the code. Though we\nencourage reviewers to be flexible with personal preferences, we do follow the\nrule \u201cTechnical facts and data overrule opinions and personal preferences\u201d.<\/p>\n\n\n\n
Enforceable changes are those which guarantee our code health and quality.<\/p>\n\n\n\n
Example of necessary changes<\/figcaption><\/figure><\/div>\n\n\n\n
Suggested changes<\/h3>\n\n\n\n
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.<\/p>\n\n\n\n
Example of suggested changes<\/figcaption><\/figure><\/div>\n\n\n\n
Informative<\/h3>\n\n\n\n
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\u2019s thoughts over the CL. They provide a path for more generic feedback which can help to build the relationship between the parts.<\/p>\n\n\n\n
Example of informatives<\/figcaption><\/figure><\/div>\n\n\n\n
In any case, all comments are intended to be\nconstructive feedback, and a way to transfer knowledge between the reviewer and\nthe reviewee, or vice-versa. There are some rules any comment should follow to\nbe effective for this purpose:<\/p>\n\n\n\n
Be kind<\/li>
Explain your reasoning<\/li>
Enable the reviewee to solve the\nproblem by just pointing out the issues with the code rather than giving an\nexplicit solution<\/li>
If the code is hard to follow,\nkindly ask to simplify the code rather than explaining the code to you<\/li><\/ul>\n\n\n\n
Building stronger teams<\/h2>\n\n\n\n
At ZEISS, we have a strong team culture. Teams\nare empowered, autonomous and fully functional units. To maintain them\nproductive we make sure there is respect and trust among team members, but\nalso, that they have fun while working on the projects.<\/p>\n\n\n\n
Code reviews as discussed play a crucial role\nin supporting the individual developer\u2019s growth and learning, but it also helps\nbuild better relationships and trust. And this is true not only for my own team\nmembers but also for people among different teams in the organization.<\/p>\n\n\n\n
![\"A](\"https:\/\/blogs.zeiss.com\/tech\/wp-content\/uploads\/2020\/08\/necessary.jpg\")
![\"A](\"https:\/\/blogs.zeiss.com\/tech\/wp-content\/uploads\/2020\/08\/Suggested.jpg\")
![\"A](\"https:\/\/blogs.zeiss.com\/tech\/wp-content\/uploads\/2020\/08\/informative.jpg\")
![\"\"](\"https:\/\/blogs.zeiss.com\/tech\/wp-content\/uploads\/2020\/08\/banner_codereviews-1024x683.jpg\")
\n <\/div>\n \n\n\n\n\n \n <\/figure>\n","protected":false},"excerpt":{"rendered":"