8

I'm now learning Gerrit (which is the first code review tool I use). Gerrit requires a reviewed change to consist of a single commit. My feature branch has about 10 commits.

The gerrit-prefered way is to squash those 10 commits into a single one. However this way if the commit will be merged into the target branch, the internal history of that feature branch will be lost. For example, I won't be able to use git-bisect to bisect into those commits. Am I right?

I am a little bit worried about this state of things. What is the rationale for this choice? Is there any way of doing this in Gerrit without losing history?

liori
  • 725

2 Answers2

2

Code review has the best impact when it is a pre-commit (pre-push in case of git) hook. If an error in encountered in 5th of your ten commits, how would you fix it (preserving history)? Sure, you can create another topic branch, fix that commit, cherry-pick the remaining 5 commits and resend the diffs for review, but this is very complex (albeit you can write a script for it).

Changes made to a single repo should ideally preserve its state (e.g. not break the build, tests, etc.) So if your changes are done this way, they can be reviewed separately, otherwise, squashing them would be better, than leaving the repo inconsistent.

You can create a bug in the bug tracker, and put a reference in every commit, so that the reviewers and future readers would know, what you were trying to achieve in whole.

vissi
  • 302
1

What about Continues Integration? you made 10 commits on a feature branch and it will be "published" at once - that would be a huge impact on others who should review those commits/changes.

However it is worth to push 10 commits, if the commits contain separate code modifications. But in case the commits contain continuous modifications on a single feature (so most of the commits just corrections or middle status of an ongoing implementation of a function definition) then better to squash them into a single commit.

In general think with the reviewers mind - what is the size of a code modification which can be reviewed easily.

Note: Gerrit is not just for reviewing the code - changes should trigger several acceptance tests. (unit test, smoke test) So if you have those, then it is harder to publish faulty codes. So from this perspective a commit should contain such changes which is worth to test.

So Gerrit force you not to commit too small and too big changes. (you will use --amend not just to correct a change already pushed to Gerrit, instead of correcting a commit you want push for review)

laplasz
  • 131