Organizing PRs and Commits
Short, simple projects produce a PR containing a single, small commit. However, we strongly encourage contributors to split up larger projects into multiple PRs or commits in order to make code reviews more effective.
This happens mainly in two situations:
A larger project should be decomposed in sub-parts / steps. We recommend one commit per logical change, up to and including when PRs are merged. See below for details.
During code review, to answer reviewer comments. We accept seeing fix-up commits during the review, but we request they are merged together (“squashed”) before PRs are merged. See Updating PRs during review | Fix up commits for more information.
Table of contents:
PR organization philosophy
A pull request (PR) is a package of work that engages three parties:
the programmer delivering the change,
the reviewer(s) who analyze and review it before it is integrated (merged) to the project,
later maintainers of the code who need to understand and modify the code further.
It is important to understand that a PR is not just about changing the code. It is really about creating a dialogue, via the history of commits, that helps both reviewers and future maintainers understanding the concepts and the components inside the code.
In a nutshell, the programmer’s job is to organize the PR to be easily reviewable and to make the code more easily maintainable. To achieve reviewability, the best way is to make each commit cover one conceptual unit of work towards the goal, and explain intent (in commit messages) separately from the code. To achieve maintainability, the programmer should explain their choices and spell out (in commit messages) what they left out, when applicable. This is explained in the section “decompose into logical changes” below.
Separately, we should also be respectful of everyone’s time. PR authors should be mindful of the reviewer’s time and help the reviewer upfront, both in terms of PR size and review guidance. This is covered in section “Help your reviewer” below. Conversely, reviewers should be respectful of PR authors. This is explained in the separate page Working as a reviewer .
Decompose PRs into individual logical changes
The main and foremost principle to organize work into PRs is “one commit per logical change”. This is explained further below, together with guidelines on how to best write commit messages. Practical suggestions on how to achieve this are further provided in the “How to” section below.
How PRs help reviewers
To do their job properly, the reviewer wants to know “what is going on” in the change. The reviewer needs to evaluate whether there is a difference between what the programmer intended to do (i.e. the goal) and what the code actually does. To do this work properly, the reviewer needs to understand what the programmer thinks they were doing—separately from the code.
It’s easy to conflate the two; programmers often like to think “my code output is, by definition, what I wanted to do”. But that is not true. To err is human; a mistake is literally a difference between intent and result, and the reviewer cannot spot mistakes if the programmer doesn’t tell them (in prose) what they wanted to do separately from the code. It’s also easy to say ”well I have a google doc which explains intent already, why do I need to explain again?”. This is unfriendly to the future maintainer of the code, who will likely not know where to find this documentation. Also, sometimes, the best reviewer comes from another team and doesn’t know about the project yet. We wish the commit messages to speak for themselves.
How PRs help maintainers
To do their job properly, the maintainers want to know “what the programmer did not put in the code”. Very often, in a project, a programmer cannot change their code in a perfect way towards the goal. The programmer makes choices, or leaves certain aspects away, on purpose, with the intent to tweak/optimize things later. The maintainers need to know what are the possible/acceptable follow-up changes, even if the code (that may be incomplete/imperfect) does not tell that on its own.
This is why we wish commit messages to tell a story that goes beyond the current change. We want the commit messages to talk about the goals, and all the ways in which the current commit/PR is not achieving the goal yet. The descriptions in prose should talk about choices (“I did X instead of Y because…” or “I did not do X even though it was requested, because…”) and link to related public issues, so that later maintainers can trace the decisions and revisit them when circumstances change.
Why each commit should correspond to one logical change
In summary, the commit messages and PR descriptions should talk about intent separately from the result, and document the choices. This does not need many sentences, or advanced use of the English language. It is not a complicated literary exercise.
It does need that the change has well-defined boundaries though. It is very had to talk about intent vs results and document choices if the commits are too fine-grained. If the programmer makes too many small commits (e.g. “fixed a comma”, or “added a function there” then another commit “added another function there”), there is nothing of value for the reviewer and maintainer to look at!
This is also wh