Learning Gerrit Code Review

4 (1 reviews total)
By Luca Milanesio
  • Instant online access to over 7,500+ books and videos
  • Constantly updated with 100+ new titles each month
  • Breadth and depth in over 1,000+ technologies

About this book

Developing software is now more than ever before a globally distributed activity: agile methodologies that worked well enough with co-located teams now need to be empowered with additional tools such as Gerrit code review to allow the developers to share, discuss, and cooperate in a more social way, even with GitHub.

Learning Gerrit Code Review is a practical guide that provides you with step-by-step instructions for the installation, configuration, and use of Gerrit code review. Using this book speeds up your adoption of Gerrit through the use of a unique, consolidated set of recipes ready to be used for LDAP authentication and to integrate Gerrit with Jenkins and GitHub.

Learning Gerrit Code Review looks at the workflow benefits of code review in an agile development team, breaks it down into simple steps, and puts it into action without any hassle. It will guide you through the installation steps of Gerrit by showing you the most typical setup and configuration schemes used in private networks.

You will also learn how to effectively use Gerrit with GitHub in order to provide the ability to add more consistent code review functionality to the social collaboration tools provided by the GitHub platform. Using the two tools together, you will be able to reuse your existing accounts and integrate your GitHub community into the development lifecycle while keeping in touch with external contributors.

Publication date:
September 2013
Publisher
Packt
Pages
144
ISBN
9781783289479

 

Chapter 1. Introducing Code Review

Welcome to the world of Code Review! In this chapter we will introduce the basic concepts of Gerrit Code Review, along with the roles and responsibilities of the development team and the actions involved when reviewing the changes pushed to the repository. We will cover the benefits of the Code Review, not only for the overall quality of the development but also the positive impacts on the entire team dynamics, collaboration and interactions between its members, and even its external counterparts. Basic terminology and a glossary of Gerrit Code Review will also be introduced in order to allow an easier and smoother introduction of the Code Review lifecycle, workflow, phases, actors, and the associated actions and concepts. By the end of this chapter we will put in place the basics for making the first steps with Gerrit.

 

Benefits of Code Review


The words "code" and "review" are sometimes a bit misleading when we try to use them to understand the fundamentals of modern Code Review. The Wikipedia definition says "Code Review is systematic examination (often known as peer review) of computer source code". This can lead to the thinking that there are two elements characterizing this operation: systematic and examination.

Historically developers believed that by being systematic it was easily expressed in terms of "coding rules" and automated through static and dynamic code analysis. Tools such as Sonar have been designed to support the definition and automation of such rules, which are then checked and validated together with your Continuous Integration environment.

Modern Code Review, however, in addition to traditional code inspection , encourages positive dynamics amongst people and facilitates collaboration, both of which cannot be replaced by automated and defined rules and processes. The most positive result of a Code Review cycle is , in addition to higher code quality standards,, it is about getting different people to look at the version of the solution and having a constructive discussion around it.

Code Review becomes part of the development process as a fundamental step and becomes a prerequisite for merging the code into its target branch.

Build stability

When adopting Continuous Integration, your goal is to get the code into your version control as soon as possible so that it can be checked in and validated with the whole project: in return you get a RAG (red-amber-green) status of your change. Whenever a build is not green, it is defined as "broken" and it is the team's priority to re-establish a healthy head version of the branch by fixing the build.

Code Review helps by reducing broken builds, thanks to the ability to link the review status to a validation step, resulting in less time wasted by the development team.

The following diagram shows Continuous Integration without Code Review:

The preceding diagram shows the typical stability of the build when every commit is directly pushed to the master branch. Whenever an incorrect patch (P1) is pushed, the build fails causing the team to intervene to fix it, which results in them pushing a new patch (P2) to re-establish a green build again. The master branch will keep a record of the failure by having both P1 and P2 in the history of the master branch.

The following diagram shows Continuous Integration with Code Review:

As shown in the preceding diagram, the introduction of Code Review allows you to check the sanity of the change in isolation, without interfering with the normal integration flow of the functionalities and keeping a green build. Even if a faulty patch is uploaded (P1), it causes a build failure only in the review branch: this can be fixed as before by submitting a new amended commit (P2) that will succeed and thus will be merged into the master.

The benefits are twofold: firstly, the normal branch stability has not been impacted and secondly, the Code Review branch has allowed the pre-validation and clean-up of the code for allowing a consistent history without losing track of the review activity and the continuous integration feedback.

Knowledge sharing

Whenever more than one person is looking at the code, the knowledge of the solution starts spreading among the team. Historically this was achieved by applying Pair Programming, one of the rules of Extreme Programming. The interaction between the "pair" leads to an earlier discussion on the solution from different perspectives with the result of a more shared understanding of the code base. This approach, however, is limited by time and space: the pair has to sit in front of the same desk at the same time, a condition that often is hard to achieve with distributed teams, which is a common set up with open source projects.

The following diagram shows Pair Programming versus Code Review:

Code Review, a practice of "team programming" is where the code is shared with potentially any member of the development team, each one free to look at it at different times beyond the code first edits timelines. The preceding diagram shows the difference in time, space, and people of the involvement of Pair-Programming versus Code Review.

There are two benefits introduced by this different approach:

  • Active involvement: This review by other people is a proposed action and is not mandatory as in pair-programming, so the people participating in the review are often keener to cooperate actively.

  • Discussion retrospective: The different opinions and discussions around the solution are stored and associated to the code: this feedback can be then analyzed and resumed at a later date, even possibly when the review is actually finished, which is particularly useful if there is a risk of team turnover.

Pair-programming however can still be used whenever a higher communication bandwidth and short turnaround of the discussion is needed, for example developing a particular complex algorithm where four focused eyes are definitely needed.

External early feedback

The possibility of having anyone reviewing your code allows external contributions from people with different mindsets and skill levels to share their views on the solution. All feedbacks are valuable, take for example, a Junior developer whose comment may be "I don't understand this fragment"—this is precious input and could prompt you to break the complexity and introduce much simpler statements. Stable code has to be simple to be maintainable: a piece of code that is obscure to a majority of developers would have more chances to be amended in the wrong way and thus to become a critical and unstable part of the project in the future! A different scenario is when we open up the review to external users or developers, including those that are accessing the code from a user perspective (that is API user, as in an Android open source project). External users can provide a different perspective on the solution providing input on its usability and the correctness and completeness of the associated documentation available. One of the fundamental aspects of Agile software development is to get feedback early in the process: with Code Review we can allow feedback when the code has not even been completed and even before having built and released it.

Shared code style

Not all programmers share the same code style, learning to read somebody else's code also means getting used to a different way of writing code and thus potentially being able to maintain it. When developers spend a significant amount of time reading other people's code, they learn not only about other parts of the project but also, consciously or not, to conform to others' coding styles.

Another more obvious way to achieve style unification is by enforcing a set of coding rules that can be defined and checked for compliance at every code build, providing an automatic negative review in case of discrepancies (Example: an open source project may require the code license to be inserted on top of each source). This type of enforcement is typically a good thing but can sometimes be seen as an additional cost or an activity that will slow down the development cycle, even if it definitely has long-terms benefits.

An indirect but effective way of achieving an agreed shared code style is to get the team just spending time reading each others code: programming languages follow the rules of natural languages, we speak and write using the same constructs of the language we hear and read every day. When a project starts with Code Review from the outset, it will naturally conform to a unique type "dialect" of language syntax shared by the developers that are working on it and reviewing it on a daily basis. It is common practice, however, to have a contributor's guide that describes the code conventions representing the "dialect" of language that has been agreed over the lifetime of the project.

Team engagement

Code Review has the power to trigger a complete set of new behaviors and dynamics among the developers. Some members naturally emerge and take the lead by commenting and proposing new creative solutions, suggesting easier frameworks or simpler algorithms for achieving the same goal. The review activity provides a natural selection of the more proactive members of the team by showing their value and appreciation through comments and commit approvals. A by-product of this is that less-senior developers are likely to push themselves to improve and try to keep up with the strongest members of the team.

The Code Review process can facilitate the team members engagement in a positive race to get the code reviewed and merged into the mainstream. It is a dynamic similar to the "CI Build game" where each successful or failed build had a corresponding score that was submitted to the last committed author. Turning the code quality selection into a positive race typically provides the benefit of engaging and encouraging people to achieve their best.

Code Review applied to open source projects becomes an excellent way to attract new team members and allows everyone to make a contribution. Having minimized the risk of breaking the build and blocking the project, allows even a new member of the project can try to express themselves by proposing a patch to the kernel of the project. Each member will also start learning from each other's feedback. The team itself could define an internal "members promotion" scheme within the project, granting new privileges based on the results of past contributions: the core team would then be automatically selected from the most involved developers, naturally elected and recognized by every member.

Qualitative code selection

Getting code under review shuffles the cards and may facilitate the adoption of a more cooperative way of planning software development cycles. Instead of forcing each change to be committed to the target code branch, you allow a natural selection and promotion of the changes that the development team collectively has judged as useful and good enough to be submitted.

This election/selection mechanism is an important new factor to the acceptance criteria of the traditional Agile planning methodologies such as scrum, where user-stories are organized in sprints, and their completion is accounted in the daily stand-up for updating the burndown chart (see the following diagram).

The following diagram shows Sprint Burndown chart compared to Review board:

When adopting Code Review, the team may decide that a completed user-story is not ready to be committed because the solution implemented does not match the quality criteria defined by its members. The flip side of the medal is that burn-down could become less predictable , as it depends on the outcome of the reviews. The methodology that emerges influences the way scrum gets executed and often drives the teams to Kanban, where the improvements to the project come from a collectively agreed set of small changes.

 

Code Review roles


In order to leverage the benefits of Gerrit, we need to assign some additional roles to the development team members. Understanding the roles, their associated duties and access permissions allows the correct insertion and responsibilities assignment in the Code Review workflow.

Contributor

With Gerrit, every user with the ability to access at least one branch of the repository in read mode and with the ability to upload a new change is known as a contributor. The philosophy behind Code Review is "getting early feedback from the team" and often there is no better feedback that just an alternative solution to the same problem: this is the reason why typically every user with read access is granted the ability to upload a change on the code.

Being a contributor does not necessarily provide that person with the ability to actually submit the code to any project branch, nor provide any guarantee that a specific change will be ever approved and merged by someone else.

There are times where changes are submitted as pure sample code (often prefixed with Request For Comments (RFC)—in their commit message) for explaining a proposal for a different implementation of an existing feature: every contribution is valuable regardless of whether the code will be effectively be used or not.

The existence of this role makes Code Review fundamentally different from the previous code analysis/inspection tools because anyone can be a contributor and provide input and value, regardless of their project history, access rights or authoritativeness about coding rules and best practices. The ability to use external contributors allows Code Review to be a fuel for innovation and project growth, especially in the open source domain.

Reviewer

In Gerrit every team member is typically allowed to express a score on a given change, expressed as a label + numeric value (positive, negative, or neutral). This role is known as a reviewer and can be further segmented according to the ability of the member to provide higher (positive or negative) score values to a change.

Team members and contributors are typically granted the ability to express a positive (+1) or negative (-1) review score in addition to simple comments on the code or on the entire change. Providing a score on a change should not be perceived as judging somebody else's skills: Code Review is always about exchanging opinions on code and should never be used for assessing team members' performance.

Note

An apparently negative review (-1) should be read as an invite for improvement, as the Gerrit meaning for it is "your code looks fine, but I would prefer you to change something before submitting it".

Committer

When team members become more experienced on the code and its underlying design, they start becoming a "technical authority" for the project. They are then entitled to provide more thorough feedback on the code and they will also have the ability to have the final word (either positive or negative) on a proposed change.

These members are known as committers and have the ability to provide a positive or negative score up to +2 (-2) for a change. The results of these reviews are absolute: +2 enables the code to be submitted into the main branch, while -2 represents a veto for the change not to be accepted.

Note

Bear in mind that Gerrit reviews are not calculative: two +1 reviews does not make a +2. Additionally, positive and negative reviews do not mutually compensate: when a -2 (veto) is given to a change it cannot be eliminated by any positive +1s or even a +2 feedback.

Typically, the number of committers is fairly limited, in the Gerrit Code Review project there are no more than six committers. This is a role that can be earned on the basis of how many changes and reviews have been successfully performed by each member during the history of the project.

Maintainer

It is sometimes useful to assign one committer the duty of administering and monitoring the project, but who then has no active role in the Code Review process. This role is called the maintainer. Their main activities are the administration of the user-to-role mapping and the assignment of the access control permissions of different roles to the project branches.

If we take as example the Gerrit Code Review OpenSource project, the maintainer is Shawn Pearce, the project founder. He has a couple of committers who have been delegated the administrative roles whenever needed.

Review labels and roles customization

Today Gerrit is a highly customizable Code Review system and all labels and concepts mentioned so far can be tailored to a project's needs using a powerful rule engine. Every review input is generally referred to as labeling with a positive/negative score. The logic behind the score can also be customized and used in project's rules for approving/rejecting the changes.

Gerrit itself uses a set of default rules for implementing the logic described so far, but a project maintainer can create additional rules and define new templates to be used in other projects.

The configuration of customized rules is beyond the scope of this book but it is covered in a set of simple "cookbook recipes" inside the Gerrit online documentation. (See https://gerrit-review.googlesource.com/Documentation/prolog-cookbook.html)

 

Review terms and workflow


Whenever a new tool is adopted, the initial problems arise when new terminologies are encountered, especially when they are referred to new concepts and methodologies. It is crucial to educate about keywords and their associated meaning in the Gerrit Code Review domain.

Project

Gerrit project is a workspace consisting of the following elements:

  • Git repository: It is used to store the merged code base and the changes under review that have not being merged yet. Gerrit has the limitation of a single repository per project. There can also be projects without any code repository associated at all (that is, Security-only projects)

  • Changes references under review: Git commit-id (expressed as SHA-1 Hexadecimal alphanumeric string) stored in the Gerrit DB and pointing to the corresponding changes stored in the Git repository

  • Access Control Lists (ACLs): It contains the list of roles defined for the Gerrit project and the associated access permissions to the Git repository branches.

  • Prolog rules: It is the set of rules that govern the Code Review process for the project.

  • Additional metadata: All the extra settings such as description, merge strategy, contributor agreements, and accessory metadata needed in order to manage the project.

Note

Gerrit supports the ability to define fine-grained access of different roles for specific branches. This feature enables there to be specifically defined levels of review depending on the criticality of the target branch (that is, experimental versus production branch).

The following diagram shows Gerrit Code Review project elements:

Change

A Gerrit change is a Git commit object uploaded for review and associated to its comments and scores. It is stored in the project's Git repository but it is not visible/accessible from the normal Git graph of commits, even it does start from a point on the commits graph.

In order for Gerrit to access the Git commit objects associated to its changes, it keeps the list of changes commit-ids in its own relational DB which are organized on a per project basis. Each change has its own unique ID, which is different to the underlying Git commit ID. The reason for this is due to the capability for a change to be amended with a set of different commits: the change id keeps a consistent link across all different amended commits.

The change id is stored at the bottom of the Git commit message into a field named Change-Id.

Gerrit changes include one or more patch-sets: all of the amended Git commits uploaded for review on the same change are stored as patch-sets where the last one represents the most recent code candidate to be merged.

Patch-sets are numbered, starting from 1 and incremented whenever a change is amended with another Git commit. It makes sense only to review only the latest patch-set as the previous ones are kept only for historical reasons, for allowing Reviewers to understand what's new from the latest inspection.

Note

A common mistake is to forget the difference between a patch-set and a change in Gerrit and, based on a negative review on an existing change, submitting a different change instead of uploading just another patch-set within the same change that received the feedback.

Label Code Review

Gerrit uses one default Code Review label with a numeric value to allow the scoring of a patch-set during a Code Review phase.

Here are the possible values and the associated meanings:

Label

Value

Meaning

Code Review

-2

Veto, code cannot be submitted in any case

-1

Code looks good but changes are needed before submission

+1

Code looks good but I need more people to review it

+2

Code looks good and can be submitted

Another common and very useful label used previously in the Android open source project is the Verified label. Since Gerrit Version 2.6 it is no longer pre-defined but it is still configurable as an additional review label with the values provided in the following table:

Label

Value

Meaning

Verified

-1

Code does not work

+1

Code works fine

Note

The combinations of Code Review/0 and Verified/0 are typically used for reviews consisting of only comments without any score.

Submit change

Submit is the action of accepting a change and requesting its incorporation into the target branch. The default set of rules for enabling a change for submission requires at a minimum a Code Review/+2.

Note

Each project may potentially have a set of different rules associated with it and expressed in terms of the project's rules.

Submitting a change does not necessarily involve the actual inclusion of it into its target branch: as Code Review happens on a forked branch it is needed to be merged or rebased to the target branch in order to be closed and incorporated.

Merge change

This is the action of merging a submitted change into its target branch. It is separated from the actual submission because of the possibilities of choosing different strategies for incorporating the change.

Bear in mind that non-fast-forward merges will generate a version of the code that could potentially be different from the one under review, depending on the merge strategy configured in Gerrit. In the case of merge conflicts, the operation will be blocked and the change will remain submitted but un-merged: this will then require the change's author or one of its Reviewers to be notified of the failure, to rebase the change and submit a new patch-set for review.

Abandon change

Whenever a change after a series of negative or inconclusive reviews does not reach the minimum requirements for being submitted, it can be flagged as abandoned so that all the team members can avoid performing any further code review on it. This typically happens because either the author gives up in trying to amend the code or just because the change has become obsolete compared to the evolution of its target branch.

Don't forget that abandoned changes are not lost or removed from the repository, so they can potentially be restored at a later stage.

 

Summary


We have introduced Gerrit Code Review and the main benefits that it brings to the development team. In addition to the pure code quality improvement and the reduction of broken builds, Code Review has a positive effect on the team's dynamic, encourages knowledge sharing, and allows an easier on-boarding of new members and their subsequent promotions.

The main roles of Gerrit Code Review are in addition to the developer/committer contributor, and reviewer often external members of the team without push permissions to the target branch. Allowing external members to provide feedback on the code improves the API contract feedback and promotes the positive growth of the product's functionality.

We have also introduced the glossary of Code Review terms that will be used throughout this book, with an initial overview of their meaning and implication in the Code Review process: now we are ready to move on to the detail of the workflow.

About the Author

  • Luca Milanesio

    Luca Milanesio is the Director and cofounder of GerritForge, the leading Git and Gerrit competence center for the enterprise. His background includes over 20 years of experience in development management, software configuration management, and software development lifecycle in large enterprises worldwide. Just prior to GerritForge LLP, Luca was the Technical Director and Senior Product Executive of the Security and Compliance platform for Electronic Payments at Primeur in Italy and UK. Since starting GerritForge LLP, Luca contributed to the Gerrit community and allowed the introduction of enterprise Code Review workflow in large enterprises worldwide including major telecoms, banks, and industries. Luca Milanesio is the author of Git Patterns and Anti-Patterns, an essential guide for scaling Git to the enterprise with 16 patterns and anti-patterns, including Hybrid SCM, Git champions, blessed repository, per-feature topic branches, and ALM integration.

    Browse publications by this author

Latest Reviews

(1 reviews total)
Good
Book Title
Access this book, plus 7,500 other titles for FREE
Access now