Skip to content

Commit a7141cd

Browse files
authored
Merge pull request astropy#6536 from astrofrog/rebase-policy
Added page to developer docs on when to rebase
2 parents 82d3736 + 6a3e140 commit a7141cd

2 files changed

Lines changed: 57 additions & 0 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
*********************************
2+
When to rebase and squash commits
3+
*********************************
4+
5+
This page describes recommendations for when to rebase pull requests and when to
6+
combine/squash commits.
7+
8+
When to remove or combine/squash commits
9+
========================================
10+
11+
Pull requests **must** be rebased and at least partially squashed (but not
12+
necessarily squashed to a single commit) if large (approximately >10KB)
13+
non-source code files (e.g. images, data files, etc.) are added and then removed
14+
or modified in the PR commit history (The squashing should remove all but the
15+
last addition of the file to not use extra space in the repository).
16+
17+
Combining/squashing commits is **encouraged** when the number of commits
18+
is excessive for the changes made. The definition of 'excessive' is
19+
subjective, but in general one should attempt to have individual commits be
20+
units of change, and not include reversions.
21+
As a concrete example, for a change affecting < 10 lines of source code and
22+
including a changelog entry, more than a few commits would be excessive.
23+
For a larger pull request adding significant functionality, however, more
24+
commits may well be appropriate.
25+
26+
As another guideline, squashing should remove extraneous information but
27+
should not be used to remove useful information for how a PR was developed. For
28+
example, 4 commits that are testing changes and have a commit message of just
29+
"debug" should be squashed. But a series of commit messages that are
30+
"Implemented feature X", "added test for feature X", "fixed bugs revealed by
31+
tests for feature X" are useful information and should not be squashed away
32+
without reason.
33+
34+
In all cases, be mindful of maintaining a welcoming environment and be helpful
35+
with advice, especially for new contributors. E.g., It is expected that a
36+
maintainer offer to help a contributor who is a novice git user do any squashing
37+
that that maintainer asks for, or do the squash themselves by directly pushing
38+
to the PR branch.
39+
40+
When to rebase
41+
==============
42+
43+
Pull requests **must** be rebased (but not necessarily squashed to a single
44+
commit) if at least one of the following conditions is met:
45+
46+
* There are conflicts with master
47+
* There are merge commits from upstream/master in the PR commit history (merge
48+
commits from PRs to the user's fork are fine)
49+
* There are commit messages include offensive language or violate the code of
50+
conduct (in this case the rebase must also edit the commit messages)
51+
52+
Github 'Squash and Merge' button
53+
================================
54+
55+
We should never use or enable the GitHub 'Squash and Merge' button since this
56+
creates problems when dealing with identifying backports.

docs/index.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ as a whole, see :doc:`development/vision`.
127127
:maxdepth: 1
128128

129129
development/workflow/development_workflow
130+
development/when_to_rebase
130131
development/codeguide
131132
development/docguide
132133
development/testguide

0 commit comments

Comments
 (0)