On Sat, Feb 18, 2012 at 2:47 AM, Andrew Otto <otto(a)wikimedia.org> wrote:
2. Do I need to rebase every time I push for review?
I don't quite understand what is going on here. I've installed git-review and am
using this to push to git. It does a rebase by default. I'm not sure if I should be
turning that off or not. Rebases seem like a bad idea unless you really need to do them.
I think git-review is doing a rebase by default so it can squash all of your local commits
into one big review commit before pushing. Yuck! This would surely mean fewer commits to
review in Gerrit, but it destroys real the history. It is making git work more like
subversion, where you just work locally until everything is good and then have one big
commit. I should be able to commit often and be able to share my commits with other
developers before having everything reviewed.
Yes, you need to rebase before you push. The rebase does not exist to
squash multiple commits into one, but to ensure that your commit can
be merged cleanly. This fits the gated trunk model, but it looks like
you don't necessarily want to gate your working branch at all, just
your master. IMO you should ask Ryan to set up direct push access for
your working branches, so you can just git push into them directly,
bypassing review. You can then merge your branch into master, and
submit that merge commit for review.
3. How does Gerrit handle merges? Do all merge
commits need to be re-approved?
Yes.
4. What should I do in the following situation?
I have a branch I recently made from master. I've made some changes and pushed them
to gerrit. My changes have been approved. Now I want to sync master into my branch. I
do
git merge master
Why did you merge master into your branch, rather than merging your
branch into master? That doesn't make much sense to me.
Then resolve any conflicts and commit. How should I
push these changes? The commits that make up the merge have already been approved in
gerrit on the master branch. Do I need to push for review using git-review? They've
already been approved, so I would think not. But gerrit will currently not allow me to
push without using git-review (is that because the commits need a Change-Id?).
Yes, you need to submit the merge commit for review. If some commits
don't have a Change-Id, git-review can't submit them, but I don't see
how that could be the case. You said the commits were already approved
in gerrit, *and* they don't have a Change-Id? Those things can't both
be true.
Since gerrit doesn't let me do a regular git push
to push my master merge to the remote branch I am tracking, I do git-review.
Perhaps
you should ask for regular pushes to be allowed if you're not
using the review workflow for that branch, see also above.
This does rebase by default, so for some reason I am
stuck having to resolve every single commit that was made to master in order to get the
merge to push. This takes quite a while, but I did it, and once the interactive rebase
was finished I was able to git-review to push the merge from master.
Great. Now I that my branch is in sync with master again, I want to merge it into
master.
git checkout master
git merge my_branch
All good. Then what? Since I can't do just 'git push', I try git-review
again. The same thing happens. I have to run through the whole interactive rebase
routine and resolve each of my commits from my_branch manually. I do that, then run
'git-review' again. Now I get this error message:
remote: Hint: A potential Change-Id was found, but it was not in the footer of the commit
message.
To ssh://otto@gerrit.wikimedia.org:29418/analytics/reportcard.git
! [remote rejected] HEAD -> refs/for/master/master (missing Change-Id in commit
message)
error: failed to push some refs to
'ssh://otto@gerrit.wikimedia.org:29418/analytics/reportcard.git'
Each of the commits I merged from my_branch come with their own Change-Id in the commit
messages. But these commits are now merge commits (I think?), so they have information
about the merge and any conflicts in the commit message below the original Change-Id. I
think this is confusing Gerrit, because it doesn't see the Change-Id in the footer.
There is a bug in git that causes merge commits to not automatically
get Change-IDs. After generating a merge commit, you need to run git
commit --amend , then save without changing anything. That makes sure
the commit-msg hook is run and the Change-ID is appended.
Roan