On Sat, Jun 9, 2018 at 9:26 AM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Sat, Jun 9, 2018 at 8:17 AM Dan Williams <dan.j.williams@xxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html