I am like most developers in that, rightly or wrongly, the most I pause before creating a pull request is to create a cleanup commit. PRs are often littered with unnecessary commits, and the commits themselves don't intentionally demonstrate the thought process behind the changes made.
There is much scope to improve. In a time when documentation is frequently trumpeted, I'm inclined to think logically grouping, ordering, and labelling the commits made in a PR can probably help more. And as it could be part of a workflow, it may be easier to stick to than an additional task that is easily forgotten like documentation.
Commit messages are the obvious starting point to improve a PR, but you can only have a good commit message if the commit changes themselves are explicative -- even with no message.
That said, it is the lowest hanging fruit -- particularly if, like me, you have created many-a-commit like
git commit -m"TKT-999 Fixes the thing"
There are already good guides for writing commit messages -- and interactive rebasing can help reword commit message before creating a PR.
There are others ways to improve PRs that aren't often discussed:
Interactive rebase can be used to manipulate commits on your branch before creating a PR. A commit that fixes a typo, for example, can be squashed. Consider this example:
$ git log commit f001584020e873aa89a3063c29c7f29fcf87317a (HEAD -> master) Author: Mark Youngman Date: Sat Apr 18 20:04:57 2020 +0100 Add even more text to test.txt commit 5ad7fb8c8a632cf36863534104fb1bfb1d27fc59 Author: Mark Youngman Date: Sat Apr 18 20:04:23 2020 +0100 Fix typo in test.txt commit 42c9d470c400f4502031dfea23977693b9973023 Author: Mark Youngman Date: Sat Apr 18 20:01:42 2020 +0100 Added more text to test.txt commit b3e17ee7b2f605482e67f812ff6fea313a74fd0f Author: Mark Youngman Date: Sat Apr 18 19:40:04 2020 +0100 Added test.txt
This can be fixed as follows:
git rebase -i HEAD~3
On starting the interactive rebase, instructions are provided:
6 # Commands: 7 # p, pick <commit> = use commit 8 # r, reword <commit> = use commit, but edit the commit message 9 # e, edit <commit> = use commit, but stop for amending 10 # s, squash <commit> = use commit, but meld into previous commit 11 # f, fixup <commit> = like "squash", but discard this commit's log message 12 # x, exec <command> = run command (the rest of the line) using shell 13 # b, break = stop here (continue rebase later with 'git rebase --continue') 14 # d, drop <commit> = remove commit 15 # l, label <label> = label current HEAD with a name 16 # t, reset <label> = reset HEAD to a label 17 # m, merge [-C <commit> | -c <commit>] <label> [# <oneline>] 18 # . create a merge commit using the original merge commit's 19 # . message (or the oneline, if no original merge commit was 20 # . specified). Use -c <commit> to reword the commit message. 21 # 22 # These lines can be re-ordered; they are executed from top to bottom. 23 # 24 # If you remove a line here THAT COMMIT WILL BE LOST. 25 # 26 # However, if you remove everything, the rebase will be aborted.
In this instance, you change pick
on the relevant commit to f
for fixup:
1 pick 42c9d47 Added more text to test.txt
2 f 5ad7fb8 Fix typo in test.txt
3 pick f001584 Add even more text to test.txt
In a different situation, evidence of the typo may still exist in your PR. To erase all evidence, you can use rebase -i
to amend the appropriate commit:
git rebase -i master [replace 'pick' on the appropriate commit with 'e'] [fix the typo] git add [file] git commit --amend git rebase --continue
After squashing unnecessary commits, a PR can be further improved by having each commit be a logical step towards the completed feature. This means the PR can be read commit by commit, showing why the feature was developed as it was.
This ordered grouping of changes often isn't the case with PRs. The steps towards the completed feature are spread amongst different commits, so rather than
Commit 1: Finish step A Commit 2: Finish step B Commit 3: Finish step C Commit 4: Finish step D
you have
Commit 1: A little bit of A and C Commit 2: Finish A and start B Commit 3: Finish B and C and D
Ideally, making your PRs more like the former than the latter is done by having the discipline to identify the steps before starting work on a feature. Back in the real world, some cleanup will be required before creating your PR.
With rebase -i
, this cleanup can largely be done by rewording, squashing, and editing commits. However, in some instances you may want to split a commit.
Changes across two commits are probably simpler to follow than both combined. For that reason, alongside padding your commit stats, splitting a commit can make sense.
A commit can be split by following these steps:
git log [make a note of the hash of the commit you wish to split] git rebase -i master [replace 'pick' to 'e' on the commit immediately _before_ the commit you wish to split] git checkout [commit_hash] -- . # Assuming you're in your project's root dir git restore --staged . [restage the files with the changes you want to appear in the 1st of the two commits] git commit -m"Your commit message" # Note we create a new commit -- not amend! git rebase --continue
If you're making such changes before creating a PR, it's probably best to clone your branch before making them, so you can check the two branches for unintentional differences.
I'm only starting to think about this, and it is uncertain to me whether time spent is worth it. But I can imagine several benefits: