Re: [GIT PULL] xen: fixes for 4.16 rc1

[Linus Torvalds]

On Fri, Feb 9, 2018 at 10:59 AM, Juergen Gross  wrote:
>
> Do you want me to setup the patches for pulling again?

No, I've pulled, I just don't want to see these unexplained merges again.

Preferably I don't want to see back-merges at all, but when I do see
them, I want to see an explanation for what they do and why they merge
that particular upstream point.

That "why that particular point" is less of an issue when you merge a
release tag (like that v4.14 merge), because at least then the point
you merge is fairly clear. But even then I want to see the "why".

We've done a really good job in the kernel of trying to have good
commit logs. It annoys me that our merge logs are sometimes really
bad.

I encourage maintainers to do

 git log --merges

and just look at the result.

I very much try to make *my* merges be informative, and - in order to
get them to that point - I obviously require help from maintainers in
the form of good pull requests. And I will literally reject pull
requests if I feel they don't have sufficiently good explanations.

My merge explanations aren't always perfect either, but on the whole I
think they at least show that I try to care. I try to format things to
be legible - often the maintainer does a great job and I can pretty
much just cut-and-paste (or take the tag message as-is), but in a lot
of those merge messages I have done a fair amount of "try to make it
legible and look good". Sometimes it's grammar and layout, and
occasionally it's me actually looking at the code and adding my own
commentary.

Now, look at merges *not* by me.

Let's just say that quality of commit messages is "varied".

Sometimes it's ok to be terse - there's a fair number of "merge topic
branch" oneliners where just the name of the topic kind of gives
things away, and I don't really complain about those (as long as the
maintainer then sent me a good explanation for *my* merge).

But sometimes even that is lacking. And for back merges, where you
aren't merging some clear development history from a topic branch of
your own or a submaintainer, the merge really *needs* an explanation.

The explanation may be "handle complex merge conflict due to XYZ so
that Linus doesn't have to", but then I really do want to see that
"XYZ" reason for whaty happened, and it really should be something
complicated, not just trivial "somebody else happened to touch
something nearby".

Or, in the case of pulling a release tag because you want to sync up,
at least _say_ so, and mention why syncing up makes things easier.

In this case, for example, those merges were actually pointless. You
merged your 4.13 tree with the 4.14 tree - which *should* have been
just a fast-forward, but I think you got a real merge just because of
the signed tag.  But you should have looked at the merge, and realized
you didn't actually have any work, and told yourself "I shouldn't
_merge_ v4.14, I should just update to it!"

So one of the reasons I want to see explanations for merges is
literally to avoid the _mindless_ merges. If you had had a policy to
write an explanation about _why_ that merge happened, you would
probably simply have realized that the merge shouldn't have happened
in the first place.

 Linus

Re: [GIT PULL] xen: fixes for 4.16 rc1

[Linus Torvalds]

On Fri, Feb 9, 2018 at 10:59 AM, Juergen Gross  wrote:
>
> Do you want me to setup the patches for pulling again?

No, I've pulled, I just don't want to see these unexplained merges again.

Preferably I don't want to see back-merges at all, but when I do see
them, I want to see an explanation for what they do and why they merge
that particular upstream point.

That "why that particular point" is less of an issue when you merge a
release tag (like that v4.14 merge), because at least then the point
you merge is fairly clear. But even then I want to see the "why".

We've done a really good job in the kernel of trying to have good
commit logs. It annoys me that our merge logs are sometimes really
bad.

I encourage maintainers to do

 git log --merges

and just look at the result.

I very much try to make *my* merges be informative, and - in order to
get them to that point - I obviously require help from maintainers in
the form of good pull requests. And I will literally reject pull
requests if I feel they don't have sufficiently good explanations.

My merge explanations aren't always perfect either, but on the whole I
think they at least show that I try to care. I try to format things to
be legible - often the maintainer does a great job and I can pretty
much just cut-and-paste (or take the tag message as-is), but in a lot
of those merge messages I have done a fair amount of "try to make it
legible and look good". Sometimes it's grammar and layout, and
occasionally it's me actually looking at the code and adding my own
commentary.

Now, look at merges *not* by me.

Let's just say that quality of commit messages is "varied".

Sometimes it's ok to be terse - there's a fair number of "merge topic
branch" oneliners where just the name of the topic kind of gives
things away, and I don't really complain about those (as long as the
maintainer then sent me a good explanation for *my* merge).

But sometimes even that is lacking. And for back merges, where you
aren't merging some clear development history from a topic branch of
your own or a submaintainer, the merge really *needs* an explanation.

The explanation may be "handle complex merge conflict due to XYZ so
that Linus doesn't have to", but then I really do want to see that
"XYZ" reason for whaty happened, and it really should be something
complicated, not just trivial "somebody else happened to touch
something nearby".

Or, in the case of pulling a release tag because you want to sync up,
at least _say_ so, and mention why syncing up makes things easier.

In this case, for example, those merges were actually pointless. You
merged your 4.13 tree with the 4.14 tree - which *should* have been
just a fast-forward, but I think you got a real merge just because of
the signed tag.  But you should have looked at the merge, and realized
you didn't actually have any work, and told yourself "I shouldn't
_merge_ v4.14, I should just update to it!"

So one of the reasons I want to see explanations for merges is
literally to avoid the _mindless_ merges. If you had had a policy to
write an explanation about _why_ that merge happened, you would
probably simply have realized that the merge shouldn't have happened
in the first place.

 Linus

Re: [GIT PULL] xen: fixes for 4.16 rc1

[Juergen Gross]

On 09/02/18 19:48, Linus Torvalds wrote:
> On Fri, Feb 9, 2018 at 6:28 AM, Juergen Gross  wrote:
>>
>>  git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
>> for-linus-4.16-rc1-tag
> 
> So I've pulled this, but the back-merges *really* annoy me.
> 
> Seriously, DON'T DO MERGES IF YOU CANNOT EVEN BE BOTHERED TO WRITE A
> REASON FOR THEM!

Sorry, I managed to screw my git tree up.

I'll modify my scripts to avoid this kind of mess.

Do you want me to setup the patches for pulling again?


Juergen

Re: [GIT PULL] xen: fixes for 4.16 rc1

[Juergen Gross]

On 09/02/18 19:48, Linus Torvalds wrote:
> On Fri, Feb 9, 2018 at 6:28 AM, Juergen Gross  wrote:
>>
>>  git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
>> for-linus-4.16-rc1-tag
> 
> So I've pulled this, but the back-merges *really* annoy me.
> 
> Seriously, DON'T DO MERGES IF YOU CANNOT EVEN BE BOTHERED TO WRITE A
> REASON FOR THEM!

Sorry, I managed to screw my git tree up.

I'll modify my scripts to avoid this kind of mess.

Do you want me to setup the patches for pulling again?


Juergen

Re: [GIT PULL] xen: fixes for 4.16 rc1

[Linus Torvalds]

On Fri, Feb 9, 2018 at 6:28 AM, Juergen Gross  wrote:
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
> for-linus-4.16-rc1-tag

So I've pulled this, but the back-merges *really* annoy me.

Seriously, DON'T DO MERGES IF YOU CANNOT EVEN BE BOTHERED TO WRITE A
REASON FOR THEM!

There are two back-merges in that tree, and while they both merge
reasonable merge points (v4.14 and v4.15 respectively), neither of
them has *any* explanation of why the f*ck the merge was done. The
v4.15 merge doesn't even make it clear that it merges v4.15 - it just
says "master".

Having random commits with no explanation of what the hell they are
doing is NOT OK. It's not ok when they do real changes, but it's EVEN
MORE not ok when they are just random merges that bring in
who-the-hell-knows-what.

Merge commits need explanations of what they merge and why they merge it. This:

Merge branch 'master' of
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/torvalds/linux

(which is all the explanation there is in commit ddb9e13af3bb) is
simply not acceptable.

At least the other merge merged the v4.14 _tag_, so just looking at
the merge it's at least clear that you merged a real release. It still
doesn't explain _why_ it was merged.

If you cannot state a good reason for the merge, or you can't be
bothered to write one, DO NOT DO THE MERGE.

It's really that simple.

  Linus "grumble" Torvalds

Re: [GIT PULL] xen: fixes for 4.16 rc1

[Linus Torvalds]

On Fri, Feb 9, 2018 at 6:28 AM, Juergen Gross  wrote:
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
> for-linus-4.16-rc1-tag

So I've pulled this, but the back-merges *really* annoy me.

Seriously, DON'T DO MERGES IF YOU CANNOT EVEN BE BOTHERED TO WRITE A
REASON FOR THEM!

There are two back-merges in that tree, and while they both merge
reasonable merge points (v4.14 and v4.15 respectively), neither of
them has *any* explanation of why the f*ck the merge was done. The
v4.15 merge doesn't even make it clear that it merges v4.15 - it just
says "master".

Having random commits with no explanation of what the hell they are
doing is NOT OK. It's not ok when they do real changes, but it's EVEN
MORE not ok when they are just random merges that bring in
who-the-hell-knows-what.

Merge commits need explanations of what they merge and why they merge it. This:

Merge branch 'master' of
ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/torvalds/linux

(which is all the explanation there is in commit ddb9e13af3bb) is
simply not acceptable.

At least the other merge merged the v4.14 _tag_, so just looking at
the merge it's at least clear that you merged a real release. It still
doesn't explain _why_ it was merged.

If you cannot state a good reason for the merge, or you can't be
bothered to write one, DO NOT DO THE MERGE.

It's really that simple.

  Linus "grumble" Torvalds