Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
1  
this might be more appropriate in stackoverflow –  GlenH7 Jun 15 '12 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. –  Andrew Finnell Jun 15 '12 at 16:17
add comment

2 Answers

up vote 2 down vote accepted

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.

share|improve this answer
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 '12 at 22:58
add comment

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)

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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