Daria Morgendorffer
Anton Salikhmetov codedot
Previous Entry Share Next Entry
Using Pull Requests as Patch Series

I should have learnt this before.

Let us suppose you have just made a pull request with about twenty commits: something borrowed, something blue, and something from Linus. Say, few commits have typos in their commit logs, few should have been merged into a single one, some should have been removed, a couple of them are so huge that you should split them into several ones, and except, maybe, one lucky change, the rest is to be completely rewritten.

In GitHub, it is however not common practice to deal with pull requests that way. After a pull request has been reviewed, it usually happens to be appended with nearly as many commits as in the original pull request. When the pull request has finally been merged, this in turn makes the master branch contain almost nothing but garbage.

Reviewing patch series and making the developer redo all the stuff from the very beginning, probably several times, might look as inapplicable approach to deal with pull requests. Besides, it does not guarantee the master branch not to contain any garbage. Of course, it is not a silver bullet. Nevertheless, it does help avoid more than 90% of junk, keeping the master branch log much more clear. That appears to be really helpful when you deal with a bug using git-bisect(1).

There exists at least the following scheme which makes GitHub-style pull requests do the trick, quite close to reviewing patch series in LKML.

  1. Make a new pull request from bug2645 to master.
  2. Discuss the changes and how to improve it until it is clear what to do for the next iteration.
  3. Close the pull request in order to save the resulting review.
  4. Fork a backup with git branch bug2645-backup bug2645 just in case.
  5. Play with git rebase -i master (edit and squash), git reset HEAD^ (splitting commits), git add -p wtf.c (s and e), and git stash -k (test results before committing) to address the comments from the review.
  6. When you are done, type git push -f origin bug2645 and start from the very beginning.

This scheme has been tested on an artificial task simulating huge and ugly patch series. Specifically, we cleared the master branch, and pretended that its backup is the development branch far away ahead of master. Then, we agreed on the rules to write commit logs in a different manner than it was before. Namely, all commit logs should have the form of 2645: update time stamps in msync(), where 2645 is the number of an issue on GitHub which corresponds to the applied changes. This way, one can always track which exactly bug implied each particular commit.

So, give it a try!

Comments Disabled:

Comments have been disabled for this post.


Log in