Code Review Workflow
This page describes a workflow for code review, through github pull requests, but that still allows for a linear commit history.
In general, we want to use the github "pull request" workflow, with a couple (very important, modifications) to keep the commit history as linear as possible.
- Don't use a fork, use a branch of the main repo
- Create a branch of each blob of work to be reviewed (format:
<AUTHOR>-<DATE>-<FEATURE>) - Try to rebase before submitting the pull request on the GitHub website
- Push changes to remote prior to issuing pull request
- Use
git prto issue the pull request (install instructions)
- Don't click the 'merge' button on GitHub!
- Use the manual instructions quoted on GitHub to pull down and merge code
- Try to always run tests before pushing the merge back to GitHub
- Try to use
--ff-only(fast-forward) forgit merge
(NB: you may need to git rebase master, then git push -f origin <branch> prior to git checkout master; git merge <branch> in order to update the remote branch in the pull request, this allows GitHub to correctly detect that the branch was rebased and merged.)
Install the tools from here:
$ git clone https://github.com/silverjam/cozyreview
$ cd cozyreview
$ ./install.shThen you can:
- Issue a pull request with
git pr - Start a review with
git review
To avoid having to use the website to issue a pull request you can use the hub tool.
$ git clone git://github.com/github/hub.git
$ cd hub
$ rake install prefix=/usr/localgit commit
git push origin my-feature-branch
hub pull-request -b <github_org>:<base_branch> -h <github_org>:<branch_to_be_pulled>
Where:
- github_org = cozybit
- base_branch = master
- branch_to_be_pulled = my-feature-branch
You can also add an alias to make your life easier, add the following aliases to ~/.gitconfig:
[alias]
cbr = rev-parse --abbrev-ref HEAD
pr = "!f() { git prb master; }; f"
prb = "!f() { patch=`tempfile --suffix=.mkd` ; echo 'PULL-REQUEST-TITLE' >>$patch ; echo >>$patch; echo '```' >>$patch ; git format-patch --stdout $1 >>$patch ; echo '```' >>$patch ; [ -z \"$EDITOR\" ] && EDITOR=vi ; $EDITOR $patch ; cbr=$(git cbr) ; hub pull-request -F $patch -b cozybit:$1 -h cozybit:$cbr ; rm -f $patch ; } ; f"
This allows you to do the following: git checkout master git checkout -b my-feature # ... make changes git commit # ... make changes git commit git push origin my-feature git pr
Commands added:
-
git pr: issue pull request for feature branches based on master -
git prb: issue pull request, but based on a branch other than master (usage:git prb other-branch)
If you want to see pull-requests as branches in git (without needing hub) you can do something like:
git config --add remote.origin.fetch "+refs/pull/*/head:refs/remotes/origin/pull-request/*"
git remote update
And now you will have remote branches from origin named like:
origin/pull-request/<pull-request-num>
This leaves your .git/config with two "fetch" keys for [remote "origin"] .. like this:
[remote "origin"]
fetch = +refs/heads/*:refs/remotes/origin/*
url = [email protected]:cozybit/distro11s.git
fetch = +refs/pull/*/head:refs/remotes/origin/pull-request/*
Idea found here: https://gist.github.com/piscisaureus/3342247
You can add an alias to accomplish this too (git config --global --edit):
[alias]
apr = "config --add remote.origin.fetch \"+refs/pull/*/head:refs/remotes/origin/pull-request/*\""
Create local branch to work in, push to remote for reviewers to pull from. This branch should remain unchanged until it is merged upstream once the submitter has sent a pull request:
git clone [email protected]:cozybit/android-proximity-networking.git
cd android-proximity-networking
git checkout -b jlopex-featurex
Push the branch for reviewers to fetch:
git push origin jlopex-featurex
git commit ...
git commit ...
git commit ...
git checkout master
git pull origin master
git checkout jlopex-featurex
git rebase --interactive master
git push origin jlopex-featurex
git request-pull origin/master origin jlopex-featurex | mailclient reviewers@project
- Select the new branch on GitHub
- Click on the pull request tab
- Send a pull request from the feature branch to master
{{ Note: this is not the recommended workflow, since it involves the added complexity of a github fork, people with direct push access to the "upstream" repo should not use a fork. }}
This illustrates the workflow needed for a code author using a fork. This is more complicated because the author must track and pull in changes from the upstream repo.
github fork git://project-upstream project-my-remote-fork
git clone git://project-my-remote-fork project-my-fork
cd project-my-fork
git remote add upstream git://project-upstream
git checkout -b myfeature
git commit ...
git commit ...
git commit ...
git fetch upstream/master
git checkout master
git merge upstream/master
Then push to remote
git push origin master
git checkout myfeature
git rebase --interactive master # or: git rebase --interactive upstream/master
git push origin myfeature
git request pull upstream/master git://project-my-remote-fork | mailclient reviewers@project-upstream
git clone [email protected]:cozybit/android-proximity-networking.git
cd android-proximity-networking
git pull origin jlopex-featurex
git checkout jlopex-featurex
git rebase --interactive master
git checkout master
git merge jlopex-featurex
Review the changes... test... etc...
git push origin master
After the review + merge + push, the feature branch is not needed anymore:
git push origin :jlopex-featurex # delete remote branch
git branch -d jlopex-featurex # delete local branch