Quick review workflow

This document describes a workflow for reviewing and committing code written by others. If you need greater detail on various aspects of working in our projects, please see Git Hub Committer Workflow.

Installing hub

If this is your first time working with github, you should install the 'hub' command and use it in place of git. You can get this from https://github.com/defunkt/hub. The install instructions there specify how to install it as $HOME/bin/hub.

mkdir -p $HOME/bin
curl http://defunkt.io/hub/standalone -sLo ~/bin/hub
chmod +x ~/bin/hub

It's useful to alias hub as git, so all git commands run through hub. Put the following line in your .bashrc:

eval "$(hub alias -s)"

If $HOME/bin isn't in your $PATH, you should add it to your PATH variable in $HOME/.bashrc like so:

export PATH=$HOME/bin:$PATH

After editing your $HOME/.bashrc file, you should reload it in your current terminal:

source $HOME/.bashrc

Reviewing the commit

Ok, now we're ready to work on reviewing this commit! Get into the repo. Note that we assume below that the origin remote refers to the github.com/kijiproject/* upstream repository.

cd src/kiji/bento-cluster

Now grab a copy of the pull request locally.

git checkout https://github.com/kijiproject/bento-cluster/pull/3

This pull request is from github.com/lockoff/bento-cluster for issue CLUSTER-1; lockoff's branch is also named "CLUSTER-1". This command has created a local branch named lockoff-CLUSTER-1 that points to the same commit history as lockoff's remote "CLUSTER-1" branch, and checks it out as your working copy branch.

At this point, do what you need to do to review the code. You can comment on the pull request itself, and submit an overall review there. Check that it meets the Code Style Guidelines and Javadoc Style Guidelines and that it doesn't break the Api Compatibility requirements.

You should also make sure it builds correctly and run tests, etc.

mvn clean verify

When you're satisfied with the commit, you should add it to the master branch. We don't want to create "diamond" merges; it makes the history hard to read. If the patch is not relative to the tip of the current origin/master branch, you should rebase it there.

First, make sure you have an up-to-date version of the master branch:

git fetch origin
git checkout master
git merge --ff-only origin/master

Then rebase the original pull request off master:

git checkout lockoff-CLUSTER-1  # Get back on the issue branch
git rebase master

If this says: Current branch lockoff-CLUSTER-1 is up to date., then the rebase was unnecessary (and a no-op); we're already in the right spot. Otherwise, the patch would be moved to the top of the history. If the merge failed, use the pull request's code review capability to request that the contributor rebase his patch on the current master branch, and submit a new pull request.

If the rebase was necessary, but the merge was automatic, it's worth double-checking that mvn clean verify, etc. all work again.

Pushing the commit

Assuming that the code is now in the correct spot in the git tree, the tests pass, and you don't have issues with the code (which should be worked out using the code review tool) or the design of the feature (which should be worked out on JIRA), then you are ready to push the commit to the master branch.

First, you should amend the commit to include your signature as the committer (as distinct from the "author") of the patch. This lets others know that you have completed this review process.

git commit --amend -s

Note that we require the following of patches to commit them:

• The first line of the commit should be: FOO-1. Jira Title here. If you need to fix this, use git commit --amend --edit to revise the commit message.
• A single issue should be a single commit. Ask people to rebase and squash multiple commits into a single commit per issue.
• All patches must come from contributors who have signed a contributor license agreement. This is automatic for WibiData employees.

Now, merge it into the master branch and push to origin

git checkout master
git merge --ff-only lockoff-CLUSTER-1
git push origin master:master

Your work in your local terminal is now complete! You should finish up by:

• Going back to the original pull request, and marking it as 'closed'.
• Go to the JIRA issue at jira.kiji.org and mark the issue as "Resolved."
  • Make sure the fixVersion and the assignee are both up-to-date.