summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwm4 <wm4@nowhere>2020-05-01 01:13:20 +0200
committerwm4 <wm4@nowhere>2020-05-01 01:13:20 +0200
commit92db8731dd5ef700c7a7b96cf5f2f5aebc6d3d88 (patch)
treeb686e7a61a73f348b4bc4152c9ef883daf9662dc
parentccbb8b1c9b61f0be85b2cff7b3b37b3c88514dda (diff)
downloadmpv-92db8731dd5ef700c7a7b96cf5f2f5aebc6d3d88.tar.bz2
mpv-92db8731dd5ef700c7a7b96cf5f2f5aebc6d3d88.tar.xz
DOCS/contribute.md: add some blabla about fixup commits in pull requests
I have to say this in PR reviews all the time, so maybe I should make i explicit. In too many words of course, many, many, too many words which nobody will read, I get it, now shut up. Sneak in some subtle or not so subtle comments about how I think that github is ruining code quality.
-rw-r--r--DOCS/contribute.md16
1 files changed, 16 insertions, 0 deletions
diff --git a/DOCS/contribute.md b/DOCS/contribute.md
index e3d98f3da7..baf93d0e25 100644
--- a/DOCS/contribute.md
+++ b/DOCS/contribute.md
@@ -100,6 +100,22 @@ Split changes into multiple commits
as possible. Commits should form logical steps in development. The way you
split changes is important for code review and analyzing bugs.
+Always squash fixup commits when making changes to pull requests
+----------------------------------------------------------------
+
+- If you make fixup commits to your pull request, you should generally squash
+ them with "git rebase -i". We prefer to have pull requests in a merge
+ ready state.
+- We don't squash-merge (nor do we use github's feature that does this) because
+ pull requests with multiple commits are perfectly legitimate, and the only
+ thing that makes sense in non-trivial cases.
+- With complex pull requests, it *may* make sense to keep them separate, but
+ they should be clearly marked as such. Reviewing commits is generally easier
+ with fixups squashed.
+- Reviewers are encouraged to look at individual commits instead of github's
+ "changes from all commits" view (which just encourages bad git and review
+ practices).
+
Touching user-visible parts may require updating the mpv docs
-------------------------------------------------------------