Friday, March 30, 2018

Email: Git best practice - break up Pull Requests into smallish commits

Hi All,
We have talked about the size of commits, the size of PRs, etc. and here is an example where I’m creating a small/medium sized feature which touches a lot of parts of the code
  • environment data
  • test data
  • helper methods
  • extending existing classes
  • the test method itself

My goal here is to show an example of breaking a PR up into small, related commits so that it’s easier to code review.  The whole feature has to go in 1 PR because all the parts of this PR depends on all the other parts, and together it’s several hundred lines of code.  But if you look at the individual commits, it’s easier to see what I was trying to do.  I did an ok job with this but surely could have done better, especially the last commit, which is pretty big.

An important thing here is that just because you have changed lot of files or even if you have changed several parts of a single file, you do not have to commit everything you have changed.  Sourcetree makes it easy to “stage” files or parts of a file which will get committed when you call commit (for more on staging, see here).  This way, even if you have a lot of changes, you can still make commits that group related pieces of code together to make life easier for the code reviewer – and hopefully for all of us since the reviewer is more likely to catch any issues with your code.