DEV Community

Cover image for Clean Pull Request Diffs
Gavin Rehkemper
Gavin Rehkemper

Posted on • Originally published at gavinr.com

Clean Pull Request Diffs

When collaborating on code with others using Git, keeping the overall list of code changes (the "diff") in a pull request as small as possible - not including lots of unnecessary changes like quotes and tabbing - is an important and considerate thing to do.

Why lots of unrelated changes suck

A pull request (aka merge request) represents a request to merge a branch that has multiple outstanding commits, with each of those commits representing a variety of lines of code changes. These code changes are shown on the pull request summary and is one of the main things someone who reviews the pull request reviews.

For those reviewers, it takes time and mental energy to understand what code change you're proposing: what it intends to do, what it actually does, and if there are any obvious bugs. When you have many un-related changed lines in the diff, that forces the reviewer to also analyze which lines are "actual" changed lines and which are unnecessary and can be ignored.

Dirty diff
A diff that's hard to see what are the significant changes

It discourages (good) reviews

The review process is already difficult enough. In many situations the reviewer is taking time out of his/her busy schedule to change contexts and review your code. Changing mind contexts then fully understanding the code changes takes a lot of effort, and adding any additional challenge to this process makes it that much harder. If a reviewer sees a large PR with a bunch of unnecessary code changes, it may influence him or her to just skip the review or perform a "low value review."

Avoid it

So, create change requests that contain only the minimum necessary code changes to fix the bug or add the feature that your pull request is trying to do. Any additional lines are detrimental to the software creation process.

Top comments (1)

Collapse
 
bcouetil profile image
Benoit COUETIL πŸ’«

You advocate for not doing code formatting, I do not think this is realistic.

You know, you can also do the formatting in one commit, and the real changes in another one. Git (and tools) help people review commit by commit.