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?

3
  • 1
    this might be more appropriate in stackoverflow
    – user53019
    Jun 15, 2012 at 16:03
  • 1
    I don't have any experience with Gerrit. I have used Atlassian FishEye + Crucible and it does not have this constraint. It may be possible to add additional commits to a single review? You might have to do it manually. Jun 15, 2012 at 16:17
  • As far as I know, Gerrit does not support reviewing a branch. It only supports reviewing and approving a single patch at a time. And they claim that it's a feature, not a bug. I don't agree and I'm looking for a tool that can review a branch as a whole and atomically either approve the branch or disapprove the whole branch (and still get comments per patch / line). Oct 25, 2018 at 6:23

2 Answers 2

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.

1
  • 2
    “how would you fix it” — mistakes happen, and as for now it was enough for us to just fix the problem in any of the later commits, as long as the master branch on central server never directly pointed to any commit between bug and fix. So, in this case, I'd put a fix as eleventh commit in the reviewed branch. Is this method a bad thing?
    – liori
    Jun 15, 2012 at 22:58
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)

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.