Date:	Sun,  3 Mar 2013 13:43:00 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, X86 ML <x86@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>, Borislav Petkov <bp@...e.de>
Subject: [PATCH] doc: Hold down best practices for pull requests

From: Borislav Petkov <bp@...e.de>

Hold down some important points to pay attention to when preparing a
pull request to upper-level maintainers and/or Linus. Based on a couple
of agitated mails from Linus to maintainers and random crowd sitting
around.

Signed-off-by: Borislav Petkov <bp@...e.de>
---

Ok,

so here's what I have. I took your message from yesterday and
morphed/fleshed it out into a set of points to pay attention to when
prepping a pull request. I also sampled some more rants from last year
to get a couple more recurring issues.

I hope I've covered the most important points.

Thanks.

 Documentation/SubmittingPullRequests | 148 +++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100644 Documentation/SubmittingPullRequests

diff --git a/Documentation/SubmittingPullRequests b/Documentation/SubmittingPullRequests
new file mode 100644
index 000000000000..d123745e0cf5
--- /dev/null
+++ b/Documentation/SubmittingPullRequests
@@ -0,0 +1,148 @@
+	Trees, merges and other hints for maintainers of all levels
+	===========================================================
+
+... or how do I send my tree to Linus without him biting my head off.
+
+
+This is a collection of hints, notes, suggestions and accepted practices
+for sending pull requests to (other maintainers which then forward those
+trees to) Linus. It is loosely based on Linus' friendly and polite hints
+to maintainers on the Linux Kernel Mailing List and the idea behind it
+is to document the most apparent do's and dont's, thus saving time and
+money of everyone involved.
+
+Basically, pull requests and merge commits adhering to the guidelines
+described below should establish certain practices which make
+development history as presentable, useful and sensible as possible.
+
+People staring at it should be able to understand what went where, when
+and why, without encountering senseless merge commits and internal
+subsystem development state which don't have any bearing on the final,
+cast-in-stone history. A second, not less important reason is tree
+bisectability.
+
+Here we go:
+
+0.) Before asking any maintainer to pull a pile of patches, make damn
+well sure that said pile is stable, works, and has been extensively,
+thoroughly and to the best of your abilities, tested. There's absolutely
+no reason in rushing half-cooked patches just because your manager says
+so. Rule of thumb is: if the patches are so done that they're boringly
+missing any issues or regressions and you've almost forgotten them in
+linux-next, *then* you can send.
+
+1.) The patchset going to an upper level maintainer should NOT be based
+on some random, potentially completely broken commit in the middle of a
+merge window, or some other random point in the tree history.
+
+Tangential to that, it shouldn't contain back-merges - not to "next"
+trees, and not to a "random commit of the day" in Linus' tree.
+
+Why, you ask?
+
+Linus: "Basically, I do not want people's development trees to worry
+about some random crazy merge-window tree of the day. You should always
+pick a good starting point that makes sense (where "makes sense" is very
+much about "it's stable and well known" and just do your own thing. What
+other people do should simply not concern you over-much. You are the
+[ ... ] maintainer, and your job is not integration, it's to make sure
+that *your* work is as stable and unsurprising as possible."
+
+2.) Write a sensible, human-readable mail message explaining in terms
+understandable to outsiders what your patchset entails, maybe describe
+the high-level big picture of where your patchset fits and why. And
+do not use abbreviations - they might be clear to you and the people
+working on your subsystem but not necessarily to the rest of the world.
+Also, include the diffstat in that mail (git request-pull should take
+care of all that nicely).
+
+3.) GPG-sign your pull request and put the high-level description you've
+created into the signed tag. Of course, your key should be signed by
+fellow developers who are in the chain of trust of the receiving end of
+your pull request.
+
+4.) The URL of your pull request should contain the signed tag. Make
+sure that URL is valid and externally accessible. This is especially
+valid for the k.org crowd who get it wrong a *lot*. Also, people tend to
+forget to push the signed tag.
+
+5.) THEN AND ONLY THEN do we start worrying about possible merge issues
+your pull request could cause with upstream. It can be very helpful if
+you look into and describe them in the pull request mail, maybe even do
+an *example* merge. This merge won't normally be used but it is very
+helpful to show the person doing the merge how to resolve possible
+conflicts.
+
+Here's Linus counting the ways why you shouldn't make merges yourself:
+
+" - I'm usually a day or two behind in my merge queue anyway, partly
+because I get tons of pull requests in a short while and I just want
+to get a feel for what's going on, and partly because I tend to do
+pulls in waves of "ok, I'm going filesystems now, then I'll look at
+drivers".
+
+ - I do a *lot* of merges. I try to make them look good, and have
+*explanations* in them. And if a merge conflict happens, I want to
+know about them.
+
+ - I want to have a gut feel about what goes into the tree when. I
+know, for example, that "oops, we had a bug in ext4 that got
+discovered only after the merge, and for a while there it didn't work
+well with larger disks". When people complain, my job is often to have
+that high-level view of "ok, you're hitting this known bug". If people
+do back-merges, that basically pulls in my tree at some random point
+that I'm not even aware of, and now you mixed up *other* peoples bugs
+into your tree.
+
+ - and somewhat most importantly (for me personally): backmerges make
+history messy, and it can be a huge pain for me when I do merge
+conflict resolution, or when other people do bisects etc. It's *much*
+nicer in many ways (visually, and for bisect reasons) to try to have
+as much independent development as possible."
+
+One more from LKML history: it can happen that your merge *actually*
+breaks upstream because it refers to symbols which are being removed by
+another, previous merge. So don't merge.
+
+6.) Avoid an excessive amount of senseless cross-merges. Think of
+it this way: a merge appearing in the final git history has to mean
+something, it has to say why it is there. So, having too many pointless
+merges simply pollutes the history and devalues it. Instead, think of
+a good point to do a cross merge, do it and *document* exactly why it is
+there.
+
+7.) And while we're at it, you should *almost* *never* rebase your
+tree. More so if it has gone public and people are possibly relying on
+it to remain stable and basing their stuff on top - especially then
+you're absolutely not allowed to rebase it anymore. But that's not that
+problematic if you've adopted the incremental development model and your
+tree is at least as well-cooked as in 1) above.
+
+IOW, "rebasing has other deeper problems too, like the fact that your
+tree is no longer something that other people can depend on. That's not
+a big issue for a new architecture, but it's a big issue going forward.
+Which is why rebasing is generally even *worse* than back-merges (but
+both are basically big "just don't do that").
+
+So the rule for both rebasing and back-merging should be:
+
+ - you should have damn good reasons for it, and you should document
+them.
+
+ - you should basically *never* rebase or back-merge random commits in
+the merge window. That's *NEVER* a good idea. If you have a conflict,
+ignore it. Explain it to me when you ask me to pull, but it is *my* job
+to know "ok, I've pulled fiftynine different trees, and trees X and Y
+end up conflicting, and this is how it got resolved". Seriously.
+
+ - if you have some really long-lived development tree, and you want to
+back-merge just to not be *too* far away, back-merge to actual releases.
+Don't pull my master branch. Say "git merge v3.8" or something like
+that, and then you write a good merge message that talks about *why* you
+wanted to update to a new release."
+
+8.) After the maintainer has pulled, it is always a good idea to take a
+look at the merge and verify it has happened as you've expected it to,
+maybe even run your tests on it to double-check everything went fine.
+
+Further reading: Documentation/development-process/*
-- 
1.8.1.3.535.ga923c31

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists