diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 12e2b18bb52..6e96fd24c40 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -380,6 +380,57 @@ pull requests from other contributors yourself after having reviewed them. +Merge/squash guidelines +----------------------- + +When a PR is approved and ready to be integrated to the ``main`` branch, one has the option to *merge* the commits unchanged, or *squash* all the commits into a single commit. + +Here are some guidelines on how to proceed, based on examples of a single PR commit history: + +1. Miscellaneous commits: + + * ``Implement X`` + * ``Fix test_a`` + * ``Add myself to AUTHORS`` + * ``fixup! Fix test_a`` + * ``Update tests/test_integration.py`` + * ``Merge origin/main into PR branch`` + * ``Update tests/test_integration.py`` + + In this case, prefer to use the **Squash** merge strategy: the commit history is a bit messy (not in a derogatory way, often one just commits changes because they know the changes will eventually be squashed together), so squashing everything into a single commit is best. You must clean up the commit message, making sure it contains useful details. + +2. Separate commits related to the same topic: + + * ``Implement X`` + * ``Add myself to AUTHORS`` + * ``Update CHANGELOG for X`` + + In this case, prefer to use the **Squash** merge strategy: while the commit history is not "messy" as in the example above, the individual commits do not bring much value overall, specially when looking at the changes a few months/years down the line. + +3. Separate commits, each with their own topic (refactorings, renames, etc), but still have a larger topic/purpose. + + * ``Refactor class X in preparation for feature Y`` + * ``Remove unused method`` + * ``Implement feature Y`` + + In this case, prefer to use the **Merge** strategy: each commit is valuable on its own, even if they serve a common topic overall. Looking at the history later, it is useful to have the removal of the unused method separately on its own commit, along with more information (such as how it became unused in the first place). + +4. Separate commits, each with their own topic, but without a larger topic/purpose other than improve the code base (using more modern techniques, improve typing, removing clutter, etc). + + * ``Improve internal names in X`` + * ``Add type annotations to Y`` + * ``Remove unnecessary dict access`` + * ``Remove unreachable code due to EOL Python`` + + In this case, prefer to use the **Merge** strategy: each commit is valuable on its own, and the information on each is valuable in the long term. + + +As mentioned, those are overall guidelines, not rules cast in stone. This topic was discussed in `#12633 `_. + + +*Backport PRs* (as those created automatically from a ``backport`` label) should always be **squashed**, as they preserve the original PR author. + + Backporting bug fixes for the next patch release ------------------------------------------------ @@ -438,6 +489,8 @@ above? All the above are not rules, but merely some guidelines/suggestions on what we should expect about backports. +Backports should be **squashed** (rather than **merged**), as doing so preserves the original PR author correctly. + Handling stale issues/PRs ------------------------- @@ -485,7 +538,7 @@ When closing a Pull Request, it needs to be acknowledging the time, effort, and -Closing Issues +Closing issues -------------- When a pull request is submitted to fix an issue, add text like ``closes #XYZW`` to the PR description and/or commits (where ``XYZW`` is the issue number). See the `GitHub docs `_ for more information.