Message-ID: <20180406011147.GD12349@sirena.org.uk>
Date:   Fri, 6 Apr 2018 02:11:47 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Takashi Iwai <tiwai@...e.de>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] sound updates for 4.17-rc1

On Thu, Apr 05, 2018 at 12:32:52PM -0700, Linus Torvalds wrote:

> The whole idea with a topic branch is that it's a topic branch. You do
> the development on that branch, and then at the END of development you
> merge it once and that branch is now *dead*. If you continue using
> that branch, you shouldn't be merging it.

Right, so even with things that aren't just "all the development on
driver X" stuff I do fairly often get cases where there's ongoing
development, for example where things have been split up for ease of
review but each series is still useful in itself.  If I'm creating topic
branches less aggressively it'd usually be either for cross tree merges
or because I expect further development in that area it'd be useful to
group together for posterity.  You seem to be saying don't do the latter
which is fine so long as I know it.

> So nobody ever sees empty merges, because by definition they never
> happen.  You only merge a branch once, and obviously only when it
> actually had real changes in it.

Like I say I'm pretty sure all the merges there now are bringing some
content in so that at least shouldn't have been an issue.

> And if you do daily merges for testing, then do them into a *testing*
> branch. Those daily merges have nothing what-so-ever to do with what
> you should send upstream, they are for testing.

> In short: do not do any automated merges for upstreaming AT ALL.
> Automated merging for that is WRONG. Don't do it.

OK.  The way I've been thinking about this has been that I'm doing the
daily merges for testing and then what I have for upstream is a snapshot
of something that had successful testing, I try to avoid modifying
whatever the last thing in -next was.  

>  (1) testing (daily, weekly, hourly, "on demand", "when I feel like
> it", or for linux-next, or whatever).

...

>      The more of this kind of automation you do the better. Do it
> daily. Do it every hour. Have a test-farm that does a build every
> single time any branch changes at all. Whatever floats your boat.
> 
>      And nobody will ever complain, because nobody will ever even
> *see* it. Except perhaps in the sense of "Wow, Mark really does a
> great job, and all his branches just always work", because all the
> daily testing you do means that you see any problems long before
> anybody else sees it.

Some of the test automation people occasionally complain about the load
I put on their systems already.

>  (2) if you have an EXPLICIT REASON to merge

>      You have two branches that have an actual dependency, or some
> other interaction, and you want to merge them together.

>      THIS IS NEVER EVER AUTOMATED!

>      It shouldn't even be scripted, because this shouldn't be about
> tens of branches anyway, and it certainly shouldn't be something all
> that common, or something is seriously wrong.

>      And btw, in this case the merge commit message should talk about
> what the dependency was, which is another reason why this shouldn't be
> scripted or automated.

Indeed, I'm doing this already except I rarely manually write a commit
message (90% of them are relatively obvious "pull in X API so we can use
it" or "merge up the fixes branch" stuff but yeah, could do better).
The merges are only ever generated by hand.

>  (3) if you are done and ready with a branch, and it's time to just
> say "the development on this branch is all done, now I will ask
> upstream to pull it, and I'll merge this into my upstream branch"

>      THIS IS NEVER AUTOMATED!

>      Sure, you might script it ("I have 35+ branches that I will send
> out, I use a script to merge them all together"), but the act of
> running that script is not something daily, it's something like "I am
> now ready to ask Takashi to take this", so you do it before you do the
> pull request to upstream.

Hrm, OK.  This I find unclear in that if you're saying it's OK to script
the merge of lots of branches I'm having trouble seeing the distinction
between that and the test merges you're talking about.  Any verbiage in
the individual merge commits if they're done en masse is going to at
best be very formulaic unless I do something like use git notes to store
a brief blurb about the topic of the branch (and even then it's likely
to get quite repetitive).  I'm seeing merge commits from other people
that look rather like that may be happening, and others that look like
they're pretty much just using git shortlog as the merge message.

I've been approaching things from the point of view that I merge
everything and then I write a signed tag explaining what that was all
about, I'd really like to be able to roll up the entire transaction but
the way to do that is a cthulhu merge which has other issues.

> You have a lot of merges that have no actual merge commit message at
> all. They say what they do, but they don't explain it. That should be
> a big warning sign to you.

It really doesn't set off alarm bells for me when reading logs, perhaps
as a result of having worked on things outside upstream (and outside the
kernel) where there were a lot of pretty routine and mechanical process
merges going on.  

> That's ok for automated testing (like pushing to linux-next or 0day
> build-bot or whatever). But those things have *nothing* to do with
> upstreaming, except in the indirect sense ("I've done a lot of testing
> on these changes, so now I'm happy to merge them for upstreaming").

> See the difference?

Honestly what I'm getting from this is that I should try to minimize the
number of topic branches I create since anything that generates a
non-trivial number of them is going to want automation which seems like
it's going to cause problems one way or another.  I appreciate that
you're talking mainly about commit messages on the merges in this mail
but the issues there are partly a product of volume and you have been
complaining about what the gitk looks like too which is definitely a
product of volume.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists