arrow_back

The "Gotcha" in Switching to Git/Gerrit for Code Review

The CI [continuous integration] pitfall to utilizing Git and Gerrit for code review.

Git is a powerful code repository and management tool - distributed, and highly flexible allowing team many ways to manage their code bases. Teams can more effectively merge from version to version, re-baseline their code with ease and deal with having many working branches of code operating at the same time.

For agile teams, however - Git alone doesn’t address code review as part of the SDLC for development. Most companies have a tool for doing those code reviews, and for Git, there is a code review tool that is open source that is often paired with Git, that tool is Gerrit. Gerrit enables you to make use of the power of Git to perform a code review before the code is added to the Git repository bound for eventual deployment, providing additional safety when making code changes. However, there is one “gotcha” in using these two tools together in your Continuous Integration environment and build processes.

 

Code Review & Continuous Integration (CI)

Let me espouse an opinion here, every single code change that you or your teammates are making to a code base should be code reviewed by another human, that human should know the code base and potentially can be part of the same team of developers.

Secondly - each and every code change should be reviewed before it makes it into the mainstream of code that is destined to be released to real users. Let me say that again; no code can make it into a release without being code reviewed, and that code review is done BEFORE it makes it into the mainline build bound for a release at some late point (seconds to days later).

 

Having that knowledge there are two places we want to be able to run continuous integration:

1) On the code being reviewed

When code is submitted to a code review, we would like to have the code pulled by the CI system and built, running the associated automated tests. This verifies that the change doesn’t break anything right now which we want to do and verify before anyone is called to do a code review. At the very basic - the code must compile, and its tests must pass for someone to feel that the should do a review in the first place.

2) On the code in the main / master branch

After a code review is complete, I would like to have my CI system verify that the code submitted to the main/master is also valid and that it passes all its automated tests.

 

Now here is where the catch comes in…

Because I want the CI system to be pulling the head constantly - and people's overall expectation is to see the thing they just merged at the ‘front’ of the build (meaning the most recent change) - you end up having to cherry pick the changes from the code review onto main/master. (Truth be told, this isn’t so much of a problem but is something that you have to work with teams to understand.)

Let's play out the scenario of doing a merge change rather than a cherry pick to see why.

  1. As a coder, I submit a bit of code for code review today.
  2. It gets reviewed and merged one week from when it was submitted.
  3. Git sees the merge as happening at a point in time BEHIND current and merges that change into that spot in the ‘change graph.’
  4. CI pulls HEAD and does a build.

Developer now looks to see if their change was included in the build eventually bound to be deployed and wonders why their change isn’t the last thing that happened. In reality, it is the last thing that happened, but Git helpfully put the change where it belonged in the change graph and all the other changes were potentially rebased to account for that change. I can hear people screaming “But that's how it is supposed to work…” and those doing so are correct but for those making the transition from other tools into Git and specifically into using Gerrit as a pre-commit review tool, this outcome can be surprising and unexpected.

Now like I said nothing in this scenario is broken, and Git/Gerrit are working exactly as they should be - it is only the developer and enterprise expectation that has been violated. While this may be sufficient for small companies, it will be more difficult for larger organizations seeking to change to Git and Gerrit to swallow this difference.

In the end, whichever way you choose to use Git/Gerrit - be it cherry pick or merge is less important than the code review portion - you should be aware that the choice to cherry pick vs. merge does have a perceived impact on those that interact with your system.

 


To learn more about automated testing, and to become an ICAgile Certified Professional in Agile Testing, check out our Agile Testing Training.