Message-ID: <CAPcyv4i9Oh4dZ0wSSxbeuVpSj+Dko1jiGaWbQ9cyW3eXsfs2Og@mail.gmail.com>
Date:   Sat, 9 Jun 2018 09:40:41 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        Linux ACPI <linux-acpi@...r.kernel.org>
Subject: Re: [GIT PULL] libnvdimm for 4.18

On Sat, Jun 9, 2018 at 9:26 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@...el.com> wrote:
>>
>> Well, crap. I've been doing it the wrong way for a while.
>
> .. you're not the only one ;*(
>
> I only really catch it when it's very obvious, like it was now when
> the last merge was just before so it stood out like a sore thumb when
> I looked at the resulting "git log".
>
> .. or then I catch it in the (happily rare) case when a merge causes
> issues, and then I curse.
>
>> Do you have a preference for more pull requests or just splitting what
>> is now a top level tag message into a summary changelog per branch
>> when I merge the ready branches for the merge window? I had been
>> assuming that the arrangement you have with Ingo / Thomas to pull
>> individual topics was a privilege for the tip tree and not necessarily
>> something everyone that sends you pulls should be doing.
>
> Oh, the -tip tree arrangement is absolutely not a "privilege" for the
> -tip tree. In fact, there are a few other trees that I have basically
> _required_ do the same because not doing it was too messy. So now I
> pull the security subsystem trees as 4-5 different separate branches
> too, for example. (And obviously the ARM tree is the "historically
> nasty" one that hasn't actually been a problem for several years, but
> the solution was the same: split it up.
>
> So I'm perfectly happy getting more than one pull request from everybody.
>
> That said, I also don't want to get _pointless_ pull requests. I don't
> want people to split their pulls "just because". It needs to make
> sense.
>
> And I *am* perfectly happy with you doing your merges too - I actually
> like seeing submaintainers using topic branches for the different
> things they do, and do their own merges. Because if I see splits, I
> want them to be along some "broader" more generic thing, not - for
> example - some "per driver pull request".
>
> But I do ask that when people do their own merges, they actually write
> a merge message for that local merge. I will still want the pull
> request to talk about the *whole* thing I'm pulling (so you having
> details in a local small merge doesn't mean that they shouldn't be
> mentioned when then asking me to pull the whole result), so there may
> be some duplication in commit messages as things get merged "up the
> stack", but that's fine.
>
> Also, the merge message really doesn't have to be extensive. But it
> should at least talk about what the topic branch you merged was doing.
> Just a sentence or two is fine, unless there's something subtle going
> on, in which case that subtle thing needs to be explained.
>
> Examples of "something subtle going on" is when the merge isn't just
> for normal development on your own, but is (for example) merging a
> common branch that you are also sharing with some other tree, or if
> the merge is a back-merge that has some important reason for it.
> Hopefully those back-merges don't even happen in the first place, but
> if they do, I _really_ want an explanation for _why_ they happened.
>
> For a regular topic branch merge, the "why" is not needed, because
> it's "obvious". You're merging development. Then just a short sentence
> of two of what the development is. Sometimes the topic branch name
> itself *might* be enough, but usually just a _bit_ more information.
>
> So for example, in that
>
>     Merge branch 'for-4.18/mcsafe' into libnvdimm-for-next
>
> merge, even just then adding below that a few sentences saying something like
>
>  "Introduce new machine-check safe copy iterator to dax and make pmem
>   use it.
>
>   Add a test-case"
>
> would have been appreciated.
>
> It really doesn't need to be much. Just a "when people do 'git log',
> they see what each commit does without having to know the big
> picture".
>
> Because imagine doing just 'git log' on my tree. You won't see just
> the libnvdimm commits - you'll see tons of random other commits too,
> and the commits that get merged by that merge commit are *not* obvious
> (because they are probably way down deep in the history, and it's not
> so easy to find them in the output of "git log" among all the other
> thousands of  commits).
>
> So just making each commit - whether it's a regular commit or a merge
> - say what it does without the reader having to know the whole context
> really makes it much more understandable.
>
> So to re-iterate: you *can* send those branches individually to me,
> and I'll just do three merges instead of one. That works too. But
> libnvdimm isn't big enough of a subsystem for me to really necessarily
> care at that level, so you doing your own topic branch merges is
> *also* good.
>
> But when you do your topic branch merges, imagine that you are me, and
> that you're a maintainer that is merging something from a
> submaintainer, and write the commit message as if you were a bit of an
> outsider that had the submaintainer explain to you what you're
> merging.
>
> And simplifying and abstracting is good. Don't try to explain the
> commits that get merged at an individual commit level - that's what
> the commit messages _in_ those commits are for. The merge should have
> a more high-level view of what gets merged.

Thanks Linus. This is great, I'm going to collect this and some your
other advice into the Maintainer Guide so we have a central document.

Powered by blists - more mailing lists