You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 4 Next »

What is Gerrit?

The Impala project uses Gerrit for all our code reviews. Gerrit is a git-based code review tool. The Impala project Gerrit server is here.

Logging in

To authenticate with Impala's Gerrit server, you'll need a Github account. Once you have one, logging in to Gerrit is as easy as clicking 'Github Sign-in' in the top-right corner.

You will need to authorize Gerrit to access some details of your Github account, and then you should be brought back to the Gerrit homepage. Unfortunately, there's a bug in the Gerrit OAuth plugin that will send you to a page that gives a 404 error. You'll need to manually go back to https://gerrit.cloudera.org/, and at that point you should be signed in as 'Anonymous Coward (X)'.

Add your public SSH key

Once you are logged in, click your user name in the top-right corner, and go to 'Settings'. Under 'SSH Public Keys' you'll need to paste in your public key:

cat ~/.ssh/id_\{r or d\}sa.pub

While you're in the settings page, please make sure to update your profile with your name, and your e-mail address!

Configuring Gerrit as a Git remote

Gerrit works by keeping a Git repository for all the projects it maintains, including Impala. In actual fact, this repository is in some sense the 'source of truth' for the Impala project, as all other repositories, including Impala's Github repository, are mirrors replicated from Gerrit.

Submitting a patch to Gerrit is therefore exactly the same as 'pushing' a sequence of commits to a remote Git repository:

git push asf-gerrit HEAD:refs/for/master

To set up Gerrit as a git remote for your Impala repository, do the following:

cd ${IMPALA_HOME}
git remote add asf-gerrit ssh://<your-github-username>@gerrit.cloudera.org:29418/Impala-ASF

Submitting a patch

Install the Gerrit pre-commit hook

Gerrit keeps track of different patches by generating a unique Change-Id that is constant over all iterations of a patch (obviously the Git hash won't do, since that changes every time the patch changes). That Change-Id is appended to every commit message:

IMPALA-1726: Move JNI / Thrift utilities to separate header

To avoid pulling in jni.h in the codegen module (where it does not
compile), this patch moves the methods that depend on both Thrift and
JNI to a separate header where it can be included more precisely.

Change-Id: I4d97f1816b24149d9163dce59f31b2afe2642b11

Gerrit provides a "pre-commit hook" that automates Change-Id generation. Installing it is very easy:

cd ${IMPALA_HOME}
curl -o .git/hooks/commit-msg https://gerrit.cloudera.org/tools/hooks/commit-msg
chmod u+x .git/hooks/commit-msg

After the hook is installed, every commit you make will get its own Change-Id. Without it, Gerrit will reject all patches.

Rebase Your Branch

Commit all outstanding changes:

cd $IMPALA_HOME
git add --update
git commit

Rebase against asf-gerrit/master (good practice to catch conflicts introduced by recent commits)

git fetch asf-gerrit
git rebase asf-gerrit/master

You might have to resolve merge conflicts during 'git rebase'. Follow the instructions, then continue with

git rebase --continue

Squash everything down to a single commit:

git rebase -i asf-gerrit/master

Make sure to pick only the first commit and 's' (squash) following lines, then edit the commit message into something coherent. Run the git log command to see that you're exactly one commit ahead of asf-gerrit/master

Tip

I have these helpers in my .bashrc to simplify branching from the latest trunk (branch-trunk my_new_branch) and rebase against the latest trunk (rebase-trunk).

branch-master() {
  git fetch asf-gerrit && git checkout -b $1 asf-gerrit/master
}
alias "rebase-master"="git fetch asf-gerrit && git rebase -i asf-gerrit/master"

Do not push directly to asf-gerrit/master, or any other branch. This will bypass the code-review process.

Submitting a patch

To submit a patch for review, you should push it to a Gerrit-specific 'branch' on the Gerrit remote. That branch has the form refs/for/<branch-name>. For the vast majority of patches the target branch will be master, which is where all mainline development happens. During releases, release managers may push commits to a specific release branch, however most new development will happen on master.

One thing to note is that Gerrit will submit all patches between the HEAD of the target branch and your current branch's HEAD for review. So:

git push asf-gerrit HEAD:refs/for/master

... will push all commits between asf-gerrit/master and your HEAD. Please therefore make certain that you are only pushing the commits you want reviewed: every separate commit translates to an e-mail in everyone's inbox!

Merging a patch (Impala committers only!)

Once a change has a +2 and has been verified by a pre-merge verification run, it can be submitted to gerrit. Once it is in is gerrit, a committer needs to push the commit to the Apache git repository, by running:

bin/push_to_asf.py

push_to_asf.py checks the latest commits in gerrit and checks if they are in the Apache git repo. If some are not in the Apache git repo yet, it will ask you if you want to update the Apache git repo with the missing commits found in the Gerrit repo. It does not check what your local state is at all. It only compares remote Gerrit with the remote Apache repo. Keep in mind that if you are a committer, it will allow you to commit any change authored by anyone that has passed GVO.

Git and Gerrit etiquette

Contribution guidelines

Before you submit a patch, please make sure that you've read the contribution guidelines.

Squash your commits

As described above, Gerrit generates one review request per new commit. It's much easier to review everything related to a patch together in one single commit, so please make sure to squash all your related commits together before submitting. If you don't, your review requests will probably get forcibly removed

  • No labels