tools.linux.kernel.org archive mirror
 help / color / mirror / Atom feed
* b4: encouraging using the cover letter in merge commits?
@ 2020-12-18 21:32 Toke Høiland-Jørgensen
  2020-12-18 22:09 ` Konstantin Ryabitsev
  2020-12-18 22:38 ` [kernel.org users] " James Bottomley
  0 siblings, 2 replies; 26+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-18 21:32 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

Hi Konstantin

Jens, Arnaldo and I just had a twitter conversation[0] about cover
letters and merge commits. Do you think it would be possible to support
the 'include the cover letter text in the merge commit message' work
flow in b4, maybe even encourage it?

I notice b4 will already save the cover letter along with the patch mbox
file, but maybe it would be possible to automate the workflow some more?
A 'b4 merge' command that wraps 'git merge' and pre-populates the commit
message with the text from the cover letter? Or some other trick? WDYT?

-Toke

[0] https://twitter.com/toke_dk/status/1340002668346040321

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: b4: encouraging using the cover letter in merge commits?
  2020-12-18 21:32 b4: encouraging using the cover letter in merge commits? Toke Høiland-Jørgensen
@ 2020-12-18 22:09 ` Konstantin Ryabitsev
  2020-12-19 12:29   ` Toke Høiland-Jørgensen
  2020-12-18 22:38 ` [kernel.org users] " James Bottomley
  1 sibling, 1 reply; 26+ messages in thread
From: Konstantin Ryabitsev @ 2020-12-18 22:09 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

On Fri, Dec 18, 2020 at 10:32:11PM +0100, Toke Høiland-Jørgensen wrote:
> I notice b4 will already save the cover letter along with the patch mbox
> file, but maybe it would be possible to automate the workflow some more?
> A 'b4 merge' command that wraps 'git merge' and pre-populates the commit
> message with the text from the cover letter? Or some other trick? WDYT?

I've been careful to operate on the "principle of least surprise," which is
why I try not to make any changes to the git repository directly. B4 will use
git commands, but in a way that don't modify the tree (only prepopulate
objects behind the scenes, etc). For this reason, I'm wary of wrapping any
git commands directly with b4.

One way I can think of is to save the payload of the cover letter into the
.cover file, if we recognize that we're in a git repository. Then, when
performing the merge, you would run:

git merge -F .cover --edit --log branchname

This will preload the cover letter into the merge message and let you edit it
before performing the merge.

What do you think?

-K

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-18 21:32 b4: encouraging using the cover letter in merge commits? Toke Høiland-Jørgensen
  2020-12-18 22:09 ` Konstantin Ryabitsev
@ 2020-12-18 22:38 ` James Bottomley
  2020-12-19 12:34   ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 26+ messages in thread
From: James Bottomley @ 2020-12-18 22:38 UTC (permalink / raw)
  To: toke, Konstantin Ryabitsev
  Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

On Fri, 2020-12-18 at 22:32 +0100, Toke Høiland-Jørgensen via
linux.kernel.org wrote:
> Hi Konstantin
> 
> Jens, Arnaldo and I just had a twitter conversation[0] about cover
> letters and merge commits. Do you think it would be possible to
> support the 'include the cover letter text in the merge commit
> message' work flow in b4, maybe even encourage it?
> 
> I notice b4 will already save the cover letter along with the patch
> mbox file, but maybe it would be possible to automate the workflow
> some more? A 'b4 merge' command that wraps 'git merge' and pre-
> populates the commit message with the text from the cover letter? Or
> some other trick?

I think it's important to ask before we do this: why is the cover
letter relevant to a merge but not to a linear patch application (which
is what a lot of maintainers use b4 for)?  I think the answer is that
it's relevant to linear patches as well, which is why we use the Link
tag, but in that case shouldn't we be using the Link tag for merge
commits also?

We did toy with the idea of using empty commits for cover letters a
while ago but they got dropped because of the problems they cause (and
the fact that they get lost on a rebase), so it could be the answer to
why merge but not linear is because we have a vehicle for the former
but not the latter, but we should think about it first.

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: b4: encouraging using the cover letter in merge commits?
  2020-12-18 22:09 ` Konstantin Ryabitsev
@ 2020-12-19 12:29   ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 26+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-19 12:29 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Fri, Dec 18, 2020 at 10:32:11PM +0100, Toke Høiland-Jørgensen wrote:
>> I notice b4 will already save the cover letter along with the patch mbox
>> file, but maybe it would be possible to automate the workflow some more?
>> A 'b4 merge' command that wraps 'git merge' and pre-populates the commit
>> message with the text from the cover letter? Or some other trick? WDYT?
>
> I've been careful to operate on the "principle of least surprise," which is
> why I try not to make any changes to the git repository directly. B4 will use
> git commands, but in a way that don't modify the tree (only prepopulate
> objects behind the scenes, etc). For this reason, I'm wary of wrapping any
> git commands directly with b4.

Fair enough :)

> One way I can think of is to save the payload of the cover letter into the
> .cover file, if we recognize that we're in a git repository. Then, when
> performing the merge, you would run:
>
> git merge -F .cover --edit --log branchname
>
> This will preload the cover letter into the merge message and let you edit it
> before performing the merge.
>
> What do you think?

I think the above would work - just putting the text of the cover letter
(without the email headers and diffstat) into a file for easy inclusion
like you suggested would likely be helpful. I'd probably name it for the
branch, though (e.g., .cover.$BRANCH ?) so that it's possible to create
multiple branches, flip around between them, and still have the cover
file be there when merging later.

I played around with creating an alias for the merge command itself, and
while having to include the branch name in the filename rules out a
simple alias, just dropping a script like this into $PATH and naming it
"git-mergedesc":

#!/bin/bash
exec git merge -F .cover.$1 --edit "$@"

makes it possible to just run 'git mergedesc branch' and get the right
behaviour (although obviously a real script should be a bit smarter
about argument parsing and validation!).

-Toke

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-18 22:38 ` [kernel.org users] " James Bottomley
@ 2020-12-19 12:34   ` Toke Høiland-Jørgensen
  2020-12-19 17:03     ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-12-19 12:34 UTC (permalink / raw)
  To: James Bottomley, Konstantin Ryabitsev
  Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

James Bottomley <James.Bottomley@HansenPartnership.com> writes:

> On Fri, 2020-12-18 at 22:32 +0100, Toke Høiland-Jørgensen via
> linux.kernel.org wrote:
>> Hi Konstantin
>> 
>> Jens, Arnaldo and I just had a twitter conversation[0] about cover
>> letters and merge commits. Do you think it would be possible to
>> support the 'include the cover letter text in the merge commit
>> message' work flow in b4, maybe even encourage it?
>> 
>> I notice b4 will already save the cover letter along with the patch
>> mbox file, but maybe it would be possible to automate the workflow
>> some more? A 'b4 merge' command that wraps 'git merge' and pre-
>> populates the commit message with the text from the cover letter? Or
>> some other trick?
>
> I think it's important to ask before we do this: why is the cover
> letter relevant to a merge but not to a linear patch application (which
> is what a lot of maintainers use b4 for)?  I think the answer is that
> it's relevant to linear patches as well, which is why we use the Link
> tag, but in that case shouldn't we be using the Link tag for merge
> commits also?
>
> We did toy with the idea of using empty commits for cover letters a
> while ago but they got dropped because of the problems they cause (and
> the fact that they get lost on a rebase), so it could be the answer to
> why merge but not linear is because we have a vehicle for the former
> but not the latter, but we should think about it first.

I agree that the cover letter is useful more often than not and ideally
it would be included in most cases. In netdev/bpf land the maintainers
do this by always creating a merge commit when applying a multi-part
series; here's Daniel applying one of mine, for instance:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183

I personally think this practice is pretty nice, and so I was hoping
that supporting this workflow in b4 could be a way to encourage other
maintainers to take up the practice as well :)

-Toke

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 12:34   ` Toke Høiland-Jørgensen
@ 2020-12-19 17:03     ` James Bottomley
  2020-12-19 17:21       ` Jakub Kicinski
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: James Bottomley @ 2020-12-19 17:03 UTC (permalink / raw)
  To: toke, Konstantin Ryabitsev
  Cc: users, tools, Jens Axboe, Arnaldo Carvalho de Melo

On Sat, 2020-12-19 at 13:34 +0100, Toke Høiland-Jørgensen via
linux.kernel.org wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> 
> > On Fri, 2020-12-18 at 22:32 +0100, Toke Høiland-Jørgensen via
> > linux.kernel.org wrote:
> > > Hi Konstantin
> > > 
> > > Jens, Arnaldo and I just had a twitter conversation[0] about
> > > cover letters and merge commits. Do you think it would be
> > > possible to support the 'include the cover letter text in the
> > > merge commit message' work flow in b4, maybe even encourage it?
> > > 
> > > I notice b4 will already save the cover letter along with the
> > > patch mbox file, but maybe it would be possible to automate the
> > > workflow some more? A 'b4 merge' command that wraps 'git merge'
> > > and pre-populates the commit message with the text from the cover
> > > letter? Or some other trick?
> > 
> > I think it's important to ask before we do this: why is the cover
> > letter relevant to a merge but not to a linear patch application
> > (which is what a lot of maintainers use b4 for)?  I think the
> > answer is that it's relevant to linear patches as well, which is
> > why we use the Link tag, but in that case shouldn't we be using the
> > Link tag for merge commits also?
> > 
> > We did toy with the idea of using empty commits for cover letters a
> > while ago but they got dropped because of the problems they cause
> > (and the fact that they get lost on a rebase), so it could be the
> > answer to why merge but not linear is because we have a vehicle for
> > the former but not the latter, but we should think about it first.
> 
> I agree that the cover letter is useful more often than not and
> ideally it would be included in most cases. In netdev/bpf land the
> maintainers do this by always creating a merge commit when applying a
> multi-part series; here's Daniel applying one of mine, for instance:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> 
> I personally think this practice is pretty nice, and so I was hoping
> that supporting this workflow in b4 could be a way to encourage other
> maintainers to take up the practice as well :)

I've got to say that creating a spurious merge for the cover letter
looks even more tortuous than creating an empty commit.  What
advantages does this have over the existing link tag practice which is
the one that we now use instead of the empty commit?

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 17:03     ` James Bottomley
@ 2020-12-19 17:21       ` Jakub Kicinski
  2020-12-19 17:32         ` James Bottomley
  2020-12-21 19:05         ` Jason Gunthorpe
  2020-12-19 18:45       ` Jonathan Corbet
  2020-12-21 17:34       ` [tools] " Mark Brown
  2 siblings, 2 replies; 26+ messages in thread
From: Jakub Kicinski @ 2020-12-19 17:21 UTC (permalink / raw)
  To: James Bottomley
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > I agree that the cover letter is useful more often than not and
> > ideally it would be included in most cases. In netdev/bpf land the
> > maintainers do this by always creating a merge commit when applying a
> > multi-part series; here's Daniel applying one of mine, for instance:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > 
> > I personally think this practice is pretty nice, and so I was hoping
> > that supporting this workflow in b4 could be a way to encourage other
> > maintainers to take up the practice as well :)  
> 
> I've got to say that creating a spurious merge for the cover letter
> looks even more tortuous than creating an empty commit.  What
> advantages does this have over the existing link tag practice which is
> the one that we now use instead of the empty commit?

May be a chicken and an egg problem in case of other subsystems.

DaveM started creating those merge commits long before Links were 
a thing (let alone lore). That gave netdev developers the ability 
to provide a high level description of their work, reasons, goals 
in the cover letter, rather than one of the commit messages. For
a series with changes finely split for ease of review it's often
awkward to pick on which commit to put that information.

Obviously the cover letter information may be made available via 
the Link, but there's obvious value in seeing the information in 
the repo, after all we don't replace commit messages with links.

We obviously have scripts to do this, we pull the relevant info 
out of patchwork:

https://git.kernel.org/pub/scm/linux/kernel/git/dborkman/pw.git/ 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 17:21       ` Jakub Kicinski
@ 2020-12-19 17:32         ` James Bottomley
  2020-12-21 19:05         ` Jason Gunthorpe
  1 sibling, 0 replies; 26+ messages in thread
From: James Bottomley @ 2020-12-19 17:32 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 2020-12-19 at 09:21 -0800, Jakub Kicinski wrote:
> On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > I agree that the cover letter is useful more often than not and
> > > ideally it would be included in most cases. In netdev/bpf land
> > > the
> > > maintainers do this by always creating a merge commit when
> > > applying a
> > > multi-part series; here's Daniel applying one of mine, for
> > > instance:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > 
> > > I personally think this practice is pretty nice, and so I was
> > > hoping
> > > that supporting this workflow in b4 could be a way to encourage
> > > other
> > > maintainers to take up the practice as well :)  
> > 
> > I've got to say that creating a spurious merge for the cover letter
> > looks even more tortuous than creating an empty commit.  What
> > advantages does this have over the existing link tag practice which
> > is
> > the one that we now use instead of the empty commit?
> 
> May be a chicken and an egg problem in case of other subsystems.
> 
> DaveM started creating those merge commits long before Links were 
> a thing (let alone lore). That gave netdev developers the ability 
> to provide a high level description of their work, reasons, goals 
> in the cover letter, rather than one of the commit messages. For
> a series with changes finely split for ease of review it's often
> awkward to pick on which commit to put that information.

To be clear I wasn't questioning why net does this.  The proposal was
that we should all start doing this and my push back was that the link
tag we've already evolved covers it.

> Obviously the cover letter information may be made available via 
> the Link, but there's obvious value in seeing the information in 
> the repo, after all we don't replace commit messages with links.

The current linear workflow doesn't group commits, which is why the
link makes more sense because it has the grouping most trees lack.  So
I think the proposal is actually that everyone should start grouping
series with merge tags as well?

When I'm digging around in the tree, it's usually for bisection
reasons, so the series information isn't relevant but the individual
commit messages are.  I understand the series information is
historically important, which is why the link tag, but I don't see any
reason why it must be present in the actual tree as opposed to easily
accessible via a link tag.

James

> We obviously have scripts to do this, we pull the relevant info 
> out of patchwork:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/dborkman/pw.git/ 




^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 17:03     ` James Bottomley
  2020-12-19 17:21       ` Jakub Kicinski
@ 2020-12-19 18:45       ` Jonathan Corbet
  2020-12-19 18:49         ` James Bottomley
  2020-12-21 17:34       ` [tools] " Mark Brown
  2 siblings, 1 reply; 26+ messages in thread
From: Jonathan Corbet @ 2020-12-19 18:45 UTC (permalink / raw)
  To: James Bottomley
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 19 Dec 2020 09:03:36 -0800
"James Bottomley" <James.Bottomley@HansenPartnership.com> wrote:

> I've got to say that creating a spurious merge for the cover letter
> looks even more tortuous than creating an empty commit.  What
> advantages does this have over the existing link tag practice which is
> the one that we now use instead of the empty commit?

I do that too when I have a significant series with information in the
cover that seems like it should be preserved; it's easy enough to do.  I
like the merge commit because it clearly ties the cover letter with the
series of patches it describes; an empty commit or link tag wouldn't do
that.

jon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 18:45       ` Jonathan Corbet
@ 2020-12-19 18:49         ` James Bottomley
  2020-12-19 18:57           ` Jonathan Corbet
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2020-12-19 18:49 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 2020-12-19 at 11:45 -0700, Jonathan Corbet wrote:
> On Sat, 19 Dec 2020 09:03:36 -0800
> "James Bottomley" <James.Bottomley@HansenPartnership.com> wrote:
> 
> > I've got to say that creating a spurious merge for the cover letter
> > looks even more tortuous than creating an empty commit.  What
> > advantages does this have over the existing link tag practice which
> > is the one that we now use instead of the empty commit?
> 
> I do that too when I have a significant series with information in
> the cover that seems like it should be preserved; it's easy enough to
> do.  like the merge commit because it clearly ties the cover letter
> with the series of patches it describes; an empty commit or link tag
> wouldn't do that.

I'm not that keen on justifying a discarded idea.  The empty commit was
an original idea for preserving the cover letter which got discarded,
actually way before the link tag.  It wouldn't preserve series
information.

However, the link tag does preserve the cover letter and all the series
information, why do you think it doesn't?

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 18:49         ` James Bottomley
@ 2020-12-19 18:57           ` Jonathan Corbet
  2020-12-19 19:03             ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Jonathan Corbet @ 2020-12-19 18:57 UTC (permalink / raw)
  To: James Bottomley
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 19 Dec 2020 10:49:21 -0800
"James Bottomley" <James.Bottomley@HansenPartnership.com> wrote:

> However, the link tag does preserve the cover letter and all the series
> information, why do you think it doesn't?

The information is essentially there, though one step further removed from
the repository.  But it documents the series as posted, not as applied;
one expects the two to be the same most of the time, but that's not always
the case.

We're getting into minor details, though.  If The Community were to decide
somehow that link tags are The Preferred Way, I would not kick and scream
too hard before going along with it.  Unless I were in one of my screaming
moods at the time, of course.

jon

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 18:57           ` Jonathan Corbet
@ 2020-12-19 19:03             ` James Bottomley
  2020-12-19 20:48               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2020-12-19 19:03 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: toke, Konstantin Ryabitsev, users, tools, Jens Axboe,
	Arnaldo Carvalho de Melo

On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
> On Sat, 19 Dec 2020 10:49:21 -0800
> "James Bottomley" <James.Bottomley@HansenPartnership.com> wrote:
> 
> > However, the link tag does preserve the cover letter and all the
> > series information, why do you think it doesn't?
> 
> The information is essentially there, though one step further removed
> from the repository.  But it documents the series as posted, not as
> applied; one expects the two to be the same most of the time, but
> that's not always the case.

But if it's not the case the change is supposed to be documented in the
commit in square brackets.  I suppose the one interesting case is when
a commit gets dropped from a series, but that should be somewhere in
the link tag email trail.

> We're getting into minor details, though.  If The Community were to
> decide somehow that link tags are The Preferred Way, I would not kick
> and scream too hard before going along with it.  Unless I were in one
> of my screaming moods at the time, of course.

I'm not really seeking a preferred way, I'm just asking why people who
now use the link tag and linear series should change.  As long as we
can agree the link tag is fine and there's really no additional
information that needs capturing, I think we can leave it to maintainer
discretion whether they prefer merge per series or linear.

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 19:03             ` James Bottomley
@ 2020-12-19 20:48               ` Arnaldo Carvalho de Melo
  2020-12-19 21:01                 ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-19 20:48 UTC (permalink / raw)
  To: James Bottomley
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:
> On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
> > On Sat, 19 Dec 2020 10:49:21 -0800
> > "James Bottomley" <James.Bottomley@HansenPartnership.com> wrote:
> > 
> > > However, the link tag does preserve the cover letter and all the
> > > series information, why do you think it doesn't?
> > 
> > The information is essentially there, though one step further removed
> > from the repository.  But it documents the series as posted, not as
> > applied; one expects the two to be the same most of the time, but
> > that's not always the case.
> 
> But if it's not the case the change is supposed to be documented in the
> commit in square brackets.  I suppose the one interesting case is when
> a commit gets dropped from a series, but that should be somewhere in
> the link tag email trail.
> 
> > We're getting into minor details, though.  If The Community were to
> > decide somehow that link tags are The Preferred Way, I would not kick
> > and scream too hard before going along with it.  Unless I were in one
> > of my screaming moods at the time, of course.
> 
> I'm not really seeking a preferred way, I'm just asking why people who
> now use the link tag and linear series should change.  As long as we
> can agree the link tag is fine and there's really no additional
> information that needs capturing, I think we can leave it to maintainer
> discretion whether they prefer merge per series or linear.

My question is: is the information in the cover letter useful? If it is,
why not have it preserved in the main repo? The owner of such
repositories asks us to describe what is in the series, sign it, and
then this gets dropped?

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 20:48               ` Arnaldo Carvalho de Melo
@ 2020-12-19 21:01                 ` James Bottomley
  2020-12-19 21:43                   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2020-12-19 21:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

On Sat, 2020-12-19 at 17:48 -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:
> > On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
[...]
> > > We're getting into minor details, though.  If The Community were
> > > to decide somehow that link tags are The Preferred Way, I would
> > > not kick and scream too hard before going along with it.  Unless
> > > I were in one of my screaming moods at the time, of course.
> > 
> > I'm not really seeking a preferred way, I'm just asking why people
> > who now use the link tag and linear series should change.  As long
> > as we can agree the link tag is fine and there's really no
> > additional information that needs capturing, I think we can leave
> > it to maintainer discretion whether they prefer merge per series or
> > linear.
> 
> My question is: is the information in the cover letter useful? 

I think it is but it's not vital to understanding individual commits,
which should be properly described.

> If it is, why not have it preserved in the main repo?

Because the link tag supplies it and works with current linear
workflows.  To mandate storing the cover letter, people using linear
workflows have to move to a new method.

>  The owner of such repositories asks us to describe what is in the
> series, sign it, and then this gets dropped?

Um, well we don't have people sign the cover letter.  We just have it
describe the current series and its history.  Plus it doesn't get
dropped ... it's in the email history, pointed to by the link tag,
which is often a lot richer than the bare cover letter anyway.

The main point is we have two pieces of information:  The precise
description of what each commit does, which should be in the tree.  And
the historical record of how the patch series got to where it was
committed, which is in the email archives.  The the cover letter is
usually not an accurate synopsis of this, which is why I prefer link
tags to the full history, but my preference isn't strong enough to
overrule people who want to store the cover letter.

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 21:01                 ` James Bottomley
@ 2020-12-19 21:43                   ` Arnaldo Carvalho de Melo
  2020-12-19 21:57                     ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-19 21:43 UTC (permalink / raw)
  To: James Bottomley
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:
> On Sat, 2020-12-19 at 17:48 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:
> > > On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
> [...]
> > > > We're getting into minor details, though.  If The Community were
> > > > to decide somehow that link tags are The Preferred Way, I would
> > > > not kick and scream too hard before going along with it.  Unless
> > > > I were in one of my screaming moods at the time, of course.
> > > 
> > > I'm not really seeking a preferred way, I'm just asking why people
> > > who now use the link tag and linear series should change.  As long
> > > as we can agree the link tag is fine and there's really no
> > > additional information that needs capturing, I think we can leave
> > > it to maintainer discretion whether they prefer merge per series or
> > > linear.
> > 
> > My question is: is the information in the cover letter useful? 
> 
> I think it is but it's not vital to understanding individual commits,
> which should be properly described.

Agreed.
 
> > If it is, why not have it preserved in the main repo?
> 
> Because the link tag supplies it and works with current linear
> workflows.  To mandate storing the cover letter, people using linear
> workflows have to move to a new method.

But that points to outside the main repository.
 
> >  The owner of such repositories asks us to describe what is in the
> > series, sign it, and then this gets dropped?
 
> Um, well we don't have people sign the cover letter.  We just have it
> describe the current series and its history.  Plus it doesn't get
> dropped ... it's in the email history, pointed to by the link tag,
> which is often a lot richer than the bare cover letter anyway.

I agree the link tag is valuable, but it points to outside the repo.
 
> The main point is we have two pieces of information:  The precise
> description of what each commit does, which should be in the tree.  And

I often have this problem with submitters: things that should be at
individual commits are grouped in the cover letter, makes my life
harder, as I'll end up having more work to do to move that to where it
belong: individual commits.

But we are digressing, assuming what is in the cover letter is not what
should be in individual commits but has value, why not have it preserved
upstream?

- Arnaldo

> the historical record of how the patch series got to where it was
> committed, which is in the email archives.  The the cover letter is
> usually not an accurate synopsis of this, which is why I prefer link
> tags to the full history, but my preference isn't strong enough to
> overrule people who want to store the cover letter.
> 
> James
> 
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 21:43                   ` Arnaldo Carvalho de Melo
@ 2020-12-19 21:57                     ` James Bottomley
  2020-12-19 22:17                       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: James Bottomley @ 2020-12-19 21:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

On Sat, 2020-12-19 at 18:43 -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:
> > On Sat, 2020-12-19 at 17:48 -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley
> > > escreveu:
> > > > On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
> > [...]
> > > > > We're getting into minor details, though.  If The Community
> > > > > were to decide somehow that link tags are The Preferred Way,
> > > > > I would not kick and scream too hard before going along with
> > > > > it.  Unless I were in one of my screaming moods at the time,
> > > > > of course.
> > > > 
> > > > I'm not really seeking a preferred way, I'm just asking why
> > > > people who now use the link tag and linear series should
> > > > change.  As long as we can agree the link tag is fine and
> > > > there's really no additional information that needs capturing,
> > > > I think we can leave it to maintainer discretion whether they
> > > > prefer merge per series or linear.
> > > 
> > > My question is: is the information in the cover letter useful? 
> > 
> > I think it is but it's not vital to understanding individual
> > commits, which should be properly described.
> 
> Agreed.
>  
> > > If it is, why not have it preserved in the main repo?
> > 
> > Because the link tag supplies it and works with current linear
> > workflows.  To mandate storing the cover letter, people using
> > linear workflows have to move to a new method.
> 
> But that points to outside the main repository.

Yes, but I don't see this as a problem.  The whole point of having
infrastructure which dereferences msgid links is that we can use it.

If this is an argument about having all the information in the repo, I
really don't think it's worth it.  All the nuance is stored in the
email trail, so simply pointing at it seems far easier.  Also, however
carefully you harvest the cover letter and relevant details into the
merge commit, you'll always miss something sometimes.  I think even net
admits this by doing both cover letter and link tag.

> > >  The owner of such repositories asks us to describe what is in
> > > the series, sign it, and then this gets dropped?
>  
> > Um, well we don't have people sign the cover letter.  We just have
> > it describe the current series and its history.  Plus it doesn't
> > get dropped ... it's in the email history, pointed to by the link
> > tag, which is often a lot richer than the bare cover letter anyway.
> 
> I agree the link tag is valuable, but it points to outside the repo.
>  
> > The main point is we have two pieces of information:  The precise
> > description of what each commit does, which should be in the
> > tree.  And
> 
> I often have this problem with submitters: things that should be at
> individual commits are grouped in the cover letter, makes my life
> harder, as I'll end up having more work to do to move that to where
> it belong: individual commits.

Well, we tend to make them do a rewrite.  Although I have to confess a
lot of it, after upteen iterations of commit messages which reproduce
the C code in slightly different English each time, becomes "get the
series into shape and we'll write the commit text for you" (or in the
case of SCSI, Martin will rewrite the commit message for you ...).

But the danger of having the cover letter is precisely that you are
less apt to be strict about the commit message, which can be confusing
for someone else when looking for a bug because they'll be going on the
commit text.

> But we are digressing, assuming what is in the cover letter is not
> what should be in individual commits but has value, why not have it
> preserved upstream?

Because on its own it's incomplete and we have other mechanisms to keep
the full historical record.

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 21:57                     ` James Bottomley
@ 2020-12-19 22:17                       ` Arnaldo Carvalho de Melo
  2020-12-19 23:34                         ` James Bottomley
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-19 22:17 UTC (permalink / raw)
  To: James Bottomley
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

Em Sat, Dec 19, 2020 at 01:57:06PM -0800, James Bottomley escreveu:
> On Sat, 2020-12-19 at 18:43 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:
> > > On Sat, 2020-12-19 at 17:48 -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley
> > > > escreveu:
> > > > > On Sat, 2020-12-19 at 11:57 -0700, Jonathan Corbet wrote:
> > > [...]
> > > > > > We're getting into minor details, though.  If The Community
> > > > > > were to decide somehow that link tags are The Preferred Way,
> > > > > > I would not kick and scream too hard before going along with
> > > > > > it.  Unless I were in one of my screaming moods at the time,
> > > > > > of course.
> > > > > 
> > > > > I'm not really seeking a preferred way, I'm just asking why
> > > > > people who now use the link tag and linear series should
> > > > > change.  As long as we can agree the link tag is fine and
> > > > > there's really no additional information that needs capturing,
> > > > > I think we can leave it to maintainer discretion whether they
> > > > > prefer merge per series or linear.
> > > > 
> > > > My question is: is the information in the cover letter useful? 
> > > 
> > > I think it is but it's not vital to understanding individual
> > > commits, which should be properly described.
> > 
> > Agreed.
> >  
> > > > If it is, why not have it preserved in the main repo?
> > > 
> > > Because the link tag supplies it and works with current linear
> > > workflows.  To mandate storing the cover letter, people using
> > > linear workflows have to move to a new method.
> > 
> > But that points to outside the main repository.

> infrastructure which dereferences msgid links is that we can use it.
 
> If this is an argument about having all the information in the repo, I
> really don't think it's worth it.

Not all the information, just the cover letters.

> All the nuance is stored in the email trail, so simply pointing at it
> seems far easier.  Also, however carefully you harvest the cover
> letter and relevant details into the merge commit, you'll always miss
> something sometimes.  I think even net admits this by doing both cover
> letter and link tag.

I'm not arguing about harvesting all the details, just the cover
letters.
 
> > > >  The owner of such repositories asks us to describe what is in
> > > > the series, sign it, and then this gets dropped?
  
> > > Um, well we don't have people sign the cover letter.  We just have
> > > it describe the current series and its history.  Plus it doesn't
> > > get dropped ... it's in the email history, pointed to by the link
> > > tag, which is often a lot richer than the bare cover letter anyway.

> > I agree the link tag is valuable, but it points to outside the repo.
  
> > > The main point is we have two pieces of information:  The precise
> > > description of what each commit does, which should be in the
> > > tree.  And
 
> > I often have this problem with submitters: things that should be at
> > individual commits are grouped in the cover letter, makes my life
> > harder, as I'll end up having more work to do to move that to where
> > it belong: individual commits.
 
> Well, we tend to make them do a rewrite.  Although I have to confess a

I have to learn, if for nothing else to teach a 5yo not to do like his
father ;-\

I want to make things progress, to avoid making these requests for doing
what is reasonable to do over and over again to downstreamers, so I end
up doing more work than I should.

But if cover letters were somehow preserved, I would just trow my hands
up and say: at least it is preserved in the repository history...

> lot of it, after upteen iterations of commit messages which reproduce
> the C code in slightly different English each time, becomes "get the
> series into shape and we'll write the commit text for you" (or in the
> case of SCSI, Martin will rewrite the commit message for you ...).
 
I can empathise with Martin.

> But the danger of having the cover letter is precisely that you are
> less apt to be strict about the commit message, which can be confusing
> for someone else when looking for a bug because they'll be going on the
> commit text.

I'm not trying to be strict, I'm trying to preserve information, trying
to be strict is making me lose a lot of time trying to herd a lot of
cats.
 
> > But we are digressing, assuming what is in the cover letter is not
> > what should be in individual commits but has value, why not have it
> > preserved upstream?
 
> Because on its own it's incomplete and we have other mechanisms to keep
> the full historical record.

I agree that link tags points to the relevant discussion, and I hope
that where that is preserved is available as long as the main repo is
available, but that is only a hope, as it is disjoint from the main
repository, keeping such valuable information in the main repository is
still important IMHO.

Its not like having cover letters in the main repository will cause
major disruption or excessive overhead.

Best regards,

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 22:17                       ` Arnaldo Carvalho de Melo
@ 2020-12-19 23:34                         ` James Bottomley
  0 siblings, 0 replies; 26+ messages in thread
From: James Bottomley @ 2020-12-19 23:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jonathan Corbet, toke, Konstantin Ryabitsev, users, tools, Jens Axboe

On Sat, 2020-12-19 at 19:17 -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Dec 19, 2020 at 01:57:06PM -0800, James Bottomley escreveu:
> > On Sat, 2020-12-19 at 18:43 -0300, Arnaldo Carvalho de Melo wrote:
> > > But we are digressing, assuming what is in the cover letter is
> > > not what should be in individual commits but has value, why not
> > > have it preserved upstream?
>  
> > Because on its own it's incomplete and we have other mechanisms to
> > keep the full historical record.
> 
> I agree that link tags points to the relevant discussion, and I hope
> that where that is preserved is available as long as the main repo is
> available, but that is only a hope, as it is disjoint from the main
> repository, keeping such valuable information in the main repository
> is still important IMHO.

OK, so I think we see the email archives in different ways.  I see them
as a thing, like the repo, which will always be there.  Even if
kernel.org gets sold off by the LF and we lose lore, there are a bunch
of internet heritage and archive projects making sure our history will
be preserved.  The link tag, since it's based on the msgid, will always
be usable through them.

I think you see this as less certain, hence the need to save the cover
letter, so in essence this comes down to *who* should preserve the
information.

> Its not like having cover letters in the main repository will cause
> major disruption or excessive overhead.

I'm not saying no-one should do it, I'm saying preserving cover letters
in merge commits is incompatible with the current linear workflow and
that link tags with that workflow provides a superset of the
information in the cover letter so there's no reason for anyone using
the linear workflow to move to a merge commit workflow.

James



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [tools] [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 17:03     ` James Bottomley
  2020-12-19 17:21       ` Jakub Kicinski
  2020-12-19 18:45       ` Jonathan Corbet
@ 2020-12-21 17:34       ` Mark Brown
  2 siblings, 0 replies; 26+ messages in thread
From: Mark Brown @ 2020-12-21 17:34 UTC (permalink / raw)
  To: tools, James.Bottomley
  Cc: toke, Konstantin Ryabitsev, users, Jens Axboe, Arnaldo Carvalho de Melo

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Sat, Dec 19, 2020 at 09:03:36AM -0800, James Bottomley wrote:

> I've got to say that creating a spurious merge for the cover letter
> looks even more tortuous than creating an empty commit.  What
> advantages does this have over the existing link tag practice which is
> the one that we now use instead of the empty commit?

My scripting will apply serieses as branches and then record the cover
letter in the merge, I don't do this so much for recording the cover as
for the benefit of my testing - I tend to queue up things to apply and
then later have automation run through and check things before actually
merging them (anything without a cover latter gets dropped into a single
series together).  Each series gets branched off separately and then
gets merged into the branch once I'm happy with the testing.  By
splitting things out like this I'm helping ensure that if a patch has
problems it only causes issues on the individual series and there's less
stuff to to pick through and redo in case of problems.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-19 17:21       ` Jakub Kicinski
  2020-12-19 17:32         ` James Bottomley
@ 2020-12-21 19:05         ` Jason Gunthorpe
  2020-12-21 21:13           ` Michal Kubeček
  2020-12-21 21:30           ` Arnaldo Carvalho de Melo
  1 sibling, 2 replies; 26+ messages in thread
From: Jason Gunthorpe @ 2020-12-21 19:05 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: James Bottomley, toke, Konstantin Ryabitsev, users, tools,
	Jens Axboe, Arnaldo Carvalho de Melo

On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > I agree that the cover letter is useful more often than not and
> > > ideally it would be included in most cases. In netdev/bpf land the
> > > maintainers do this by always creating a merge commit when applying a
> > > multi-part series; here's Daniel applying one of mine, for instance:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > 
> > > I personally think this practice is pretty nice, and so I was hoping
> > > that supporting this workflow in b4 could be a way to encourage other
> > > maintainers to take up the practice as well :)  
> > 
> > I've got to say that creating a spurious merge for the cover letter
> > looks even more tortuous than creating an empty commit.  What
> > advantages does this have over the existing link tag practice which is
> > the one that we now use instead of the empty commit?
> 
> May be a chicken and an egg problem in case of other subsystems.
> 
> DaveM started creating those merge commits long before Links were 
> a thing (let alone lore). That gave netdev developers the ability 
> to provide a high level description of their work, reasons, goals 
> in the cover letter, rather than one of the commit messages. For
> a series with changes finely split for ease of review it's often
> awkward to pick on which commit to put that information.
> 
> Obviously the cover letter information may be made available via 
> the Link, but there's obvious value in seeing the information in 
> the repo, after all we don't replace commit messages with links.

My biggest problem with the cover letters is while the are in the
repository, someplace, I've never actually found one while hunting
around in the git history for clues, eg with 'git blame' or 'git log
log -p'

In fact more often than not I find the netdev cover letters through
hunting in lore, not through git.

Is there some git sequence to make it visible?

The Link header is a nicer because no matter how I end up at a commit
I can go back to an email discussion..

Jason

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-21 19:05         ` Jason Gunthorpe
@ 2020-12-21 21:13           ` Michal Kubeček
  2020-12-21 21:30           ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 26+ messages in thread
From: Michal Kubeček @ 2020-12-21 21:13 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Jakub Kicinski, James Bottomley, toke, Konstantin Ryabitsev,
	users, tools, Jens Axboe, Arnaldo Carvalho de Melo

On Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe wrote:
> My biggest problem with the cover letters is while the are in the
> repository, someplace, I've never actually found one while hunting
> around in the git history for clues, eg with 'git blame' or 'git log
> log -p'
> 
> In fact more often than not I find the netdev cover letters through
> hunting in lore, not through git.
> 
> Is there some git sequence to make it visible?
> 
> The Link header is a nicer because no matter how I end up at a commit
> I can go back to an email discussion..

If you have a commit in merged branch, the corresponding merge commit
should be the last shown by

  git rev-list --ancestry-path --merges --topo-order --pretty=full \
      ${id}..${branch}

Or first if you also add "--reverse". Unfortunately "-n 1" cannot be
used to show just that merge commit as it would be applied to the list
before "--reverse".

In a script, you could either postprocess the output with sed or count
the merge commits first:

  count=$(git rev-list --ancestry-path --merges --topo-order \
      "${id}..${branch}" | wc -l)
  git rev-list --ancestry-path --merges --topo-order --skip $[count - 1] \
      --pretty=full "${id}..${branch}"

Michal

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-21 19:05         ` Jason Gunthorpe
  2020-12-21 21:13           ` Michal Kubeček
@ 2020-12-21 21:30           ` Arnaldo Carvalho de Melo
  2020-12-22  6:30             ` Leon Romanovsky
  1 sibling, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-12-21 21:30 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Jakub Kicinski, James Bottomley, toke, Konstantin Ryabitsev,
	users, tools, Jens Axboe

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:
> On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> > On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > > I agree that the cover letter is useful more often than not and
> > > > ideally it would be included in most cases. In netdev/bpf land the
> > > > maintainers do this by always creating a merge commit when applying a
> > > > multi-part series; here's Daniel applying one of mine, for instance:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > > 
> > > > I personally think this practice is pretty nice, and so I was hoping
> > > > that supporting this workflow in b4 could be a way to encourage other
> > > > maintainers to take up the practice as well :)  
> > > 
> > > I've got to say that creating a spurious merge for the cover letter
> > > looks even more tortuous than creating an empty commit.  What
> > > advantages does this have over the existing link tag practice which is
> > > the one that we now use instead of the empty commit?
> > 
> > May be a chicken and an egg problem in case of other subsystems.
> > 
> > DaveM started creating those merge commits long before Links were 
> > a thing (let alone lore). That gave netdev developers the ability 
> > to provide a high level description of their work, reasons, goals 
> > in the cover letter, rather than one of the commit messages. For
> > a series with changes finely split for ease of review it's often
> > awkward to pick on which commit to put that information.
> > 
> > Obviously the cover letter information may be made available via 
> > the Link, but there's obvious value in seeing the information in 
> > the repo, after all we don't replace commit messages with links.
 
> My biggest problem with the cover letters is while the are in the
> repository, someplace, I've never actually found one while hunting
> around in the git history for clues, eg with 'git blame' or 'git log
> log -p'

Well, they don't get merged, this is the point of this thread :-\

That or they are really well hidden :-)
 
> In fact more often than not I find the netdev cover letters through
> hunting in lore, not through git.
 
> Is there some git sequence to make it visible?
 
> The Link header is a nicer because no matter how I end up at a commit
> I can go back to an email discussion..

For going back to the discussion the link is fantastic, its just the
cover letter, that is made with the specific intent to justify that
follows it (the patches) should get merged, so should, I think, be
merged _too_.

This is not about having everything in the repo or leaving as much as
possible outside, pointed by the Link: tag, not even a middle ground,
its just about the cover letter.

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-21 21:30           ` Arnaldo Carvalho de Melo
@ 2020-12-22  6:30             ` Leon Romanovsky
  2020-12-22  8:14               ` Geert Uytterhoeven
  0 siblings, 1 reply; 26+ messages in thread
From: Leon Romanovsky @ 2020-12-22  6:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jason Gunthorpe, Jakub Kicinski, James Bottomley, toke,
	Konstantin Ryabitsev, users, tools, Jens Axboe

On Mon, Dec 21, 2020 at 06:30:12PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:
> > On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> > > On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > > > I agree that the cover letter is useful more often than not and
> > > > > ideally it would be included in most cases. In netdev/bpf land the
> > > > > maintainers do this by always creating a merge commit when applying a
> > > > > multi-part series; here's Daniel applying one of mine, for instance:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > > >
> > > > > I personally think this practice is pretty nice, and so I was hoping
> > > > > that supporting this workflow in b4 could be a way to encourage other
> > > > > maintainers to take up the practice as well :)
> > > >
> > > > I've got to say that creating a spurious merge for the cover letter
> > > > looks even more tortuous than creating an empty commit.  What
> > > > advantages does this have over the existing link tag practice which is
> > > > the one that we now use instead of the empty commit?
> > >
> > > May be a chicken and an egg problem in case of other subsystems.
> > >
> > > DaveM started creating those merge commits long before Links were
> > > a thing (let alone lore). That gave netdev developers the ability
> > > to provide a high level description of their work, reasons, goals
> > > in the cover letter, rather than one of the commit messages. For
> > > a series with changes finely split for ease of review it's often
> > > awkward to pick on which commit to put that information.
> > >
> > > Obviously the cover letter information may be made available via
> > > the Link, but there's obvious value in seeing the information in
> > > the repo, after all we don't replace commit messages with links.
>
> > My biggest problem with the cover letters is while the are in the
> > repository, someplace, I've never actually found one while hunting
> > around in the git history for clues, eg with 'git blame' or 'git log
> > log -p'
>
> Well, they don't get merged, this is the point of this thread :-\

In the netdev flow, the cover letters are merged.
See commit 28f53159e121 ("Merge branch 'vsock-add-flags-field-in-the-vsock-address'")

Thanks

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-22  6:30             ` Leon Romanovsky
@ 2020-12-22  8:14               ` Geert Uytterhoeven
  2020-12-22 12:36                 ` Leon Romanovsky
  2021-01-05 13:38                 ` Jason Gunthorpe
  0 siblings, 2 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2020-12-22  8:14 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Arnaldo Carvalho de Melo, Jason Gunthorpe, Jakub Kicinski,
	James Bottomley, toke, Konstantin Ryabitsev, users, tools,
	Jens Axboe, Michal Kubecek

Hi Leon,

On Tue, Dec 22, 2020 at 7:30 AM Leon Romanovsky <leon@kernel.org> wrote:
> On Mon, Dec 21, 2020 at 06:30:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:
> > > On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> > > > On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > > > > I agree that the cover letter is useful more often than not and
> > > > > > ideally it would be included in most cases. In netdev/bpf land the
> > > > > > maintainers do this by always creating a merge commit when applying a
> > > > > > multi-part series; here's Daniel applying one of mine, for instance:
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > > > >
> > > > > > I personally think this practice is pretty nice, and so I was hoping
> > > > > > that supporting this workflow in b4 could be a way to encourage other
> > > > > > maintainers to take up the practice as well :)
> > > > >
> > > > > I've got to say that creating a spurious merge for the cover letter
> > > > > looks even more tortuous than creating an empty commit.  What
> > > > > advantages does this have over the existing link tag practice which is
> > > > > the one that we now use instead of the empty commit?
> > > >
> > > > May be a chicken and an egg problem in case of other subsystems.
> > > >
> > > > DaveM started creating those merge commits long before Links were
> > > > a thing (let alone lore). That gave netdev developers the ability
> > > > to provide a high level description of their work, reasons, goals
> > > > in the cover letter, rather than one of the commit messages. For
> > > > a series with changes finely split for ease of review it's often
> > > > awkward to pick on which commit to put that information.
> > > >
> > > > Obviously the cover letter information may be made available via
> > > > the Link, but there's obvious value in seeing the information in
> > > > the repo, after all we don't replace commit messages with links.
> >
> > > My biggest problem with the cover letters is while the are in the
> > > repository, someplace, I've never actually found one while hunting
> > > around in the git history for clues, eg with 'git blame' or 'git log
> > > log -p'
> >
> > Well, they don't get merged, this is the point of this thread :-\
>
> In the netdev flow, the cover letters are merged.
> See commit 28f53159e121 ("Merge branch 'vsock-add-flags-field-in-the-vsock-address'")

But "git log -p -- net/vmw_vsock/af_vsock.c" does not show it.
You need convoluted commands like Michal posted. And those don't
take the path directly, so you have to find ${id} and $[branch} first.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-22  8:14               ` Geert Uytterhoeven
@ 2020-12-22 12:36                 ` Leon Romanovsky
  2021-01-05 13:38                 ` Jason Gunthorpe
  1 sibling, 0 replies; 26+ messages in thread
From: Leon Romanovsky @ 2020-12-22 12:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Arnaldo Carvalho de Melo, Jason Gunthorpe, Jakub Kicinski,
	James Bottomley, toke, Konstantin Ryabitsev, users, tools,
	Jens Axboe, Michal Kubecek

On Tue, Dec 22, 2020 at 09:14:48AM +0100, Geert Uytterhoeven wrote:
> Hi Leon,
>
> On Tue, Dec 22, 2020 at 7:30 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Mon, Dec 21, 2020 at 06:30:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:
> > > > On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> > > > > On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > > > > > I agree that the cover letter is useful more often than not and
> > > > > > > ideally it would be included in most cases. In netdev/bpf land the
> > > > > > > maintainers do this by always creating a merge commit when applying a
> > > > > > > multi-part series; here's Daniel applying one of mine, for instance:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > > > > >
> > > > > > > I personally think this practice is pretty nice, and so I was hoping
> > > > > > > that supporting this workflow in b4 could be a way to encourage other
> > > > > > > maintainers to take up the practice as well :)
> > > > > >
> > > > > > I've got to say that creating a spurious merge for the cover letter
> > > > > > looks even more tortuous than creating an empty commit.  What
> > > > > > advantages does this have over the existing link tag practice which is
> > > > > > the one that we now use instead of the empty commit?
> > > > >
> > > > > May be a chicken and an egg problem in case of other subsystems.
> > > > >
> > > > > DaveM started creating those merge commits long before Links were
> > > > > a thing (let alone lore). That gave netdev developers the ability
> > > > > to provide a high level description of their work, reasons, goals
> > > > > in the cover letter, rather than one of the commit messages. For
> > > > > a series with changes finely split for ease of review it's often
> > > > > awkward to pick on which commit to put that information.
> > > > >
> > > > > Obviously the cover letter information may be made available via
> > > > > the Link, but there's obvious value in seeing the information in
> > > > > the repo, after all we don't replace commit messages with links.
> > >
> > > > My biggest problem with the cover letters is while the are in the
> > > > repository, someplace, I've never actually found one while hunting
> > > > around in the git history for clues, eg with 'git blame' or 'git log
> > > > log -p'
> > >
> > > Well, they don't get merged, this is the point of this thread :-\
> >
> > In the netdev flow, the cover letters are merged.
> > See commit 28f53159e121 ("Merge branch 'vsock-add-flags-field-in-the-vsock-address'")
>
> But "git log -p -- net/vmw_vsock/af_vsock.c" does not show it.
> You need convoluted commands like Michal posted. And those don't
> take the path directly, so you have to find ${id} and $[branch} first.

I agree, just wanted to give a contra-example to the sentence "they don't get merged".

Thanks

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [kernel.org users] b4: encouraging using the cover letter in merge commits?
  2020-12-22  8:14               ` Geert Uytterhoeven
  2020-12-22 12:36                 ` Leon Romanovsky
@ 2021-01-05 13:38                 ` Jason Gunthorpe
  1 sibling, 0 replies; 26+ messages in thread
From: Jason Gunthorpe @ 2021-01-05 13:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Leon Romanovsky, Arnaldo Carvalho de Melo, Jakub Kicinski,
	James Bottomley, toke, Konstantin Ryabitsev, users, tools,
	Jens Axboe, Michal Kubecek

On Tue, Dec 22, 2020 at 09:14:48AM +0100, Geert Uytterhoeven wrote:
> Hi Leon,
> 
> On Tue, Dec 22, 2020 at 7:30 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Mon, Dec 21, 2020 at 06:30:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:
> > > > On Sat, Dec 19, 2020 at 09:21:26AM -0800, Jakub Kicinski wrote:
> > > > > On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
> > > > > > > I agree that the cover letter is useful more often than not and
> > > > > > > ideally it would be included in most cases. In netdev/bpf land the
> > > > > > > maintainers do this by always creating a merge commit when applying a
> > > > > > > multi-part series; here's Daniel applying one of mine, for instance:
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=4e083fdfa39db29bbc7725e229e701867d0da183
> > > > > > >
> > > > > > > I personally think this practice is pretty nice, and so I was hoping
> > > > > > > that supporting this workflow in b4 could be a way to encourage other
> > > > > > > maintainers to take up the practice as well :)
> > > > > >
> > > > > > I've got to say that creating a spurious merge for the cover letter
> > > > > > looks even more tortuous than creating an empty commit.  What
> > > > > > advantages does this have over the existing link tag practice which is
> > > > > > the one that we now use instead of the empty commit?
> > > > >
> > > > > May be a chicken and an egg problem in case of other subsystems.
> > > > >
> > > > > DaveM started creating those merge commits long before Links were
> > > > > a thing (let alone lore). That gave netdev developers the ability
> > > > > to provide a high level description of their work, reasons, goals
> > > > > in the cover letter, rather than one of the commit messages. For
> > > > > a series with changes finely split for ease of review it's often
> > > > > awkward to pick on which commit to put that information.
> > > > >
> > > > > Obviously the cover letter information may be made available via
> > > > > the Link, but there's obvious value in seeing the information in
> > > > > the repo, after all we don't replace commit messages with links.
> > >
> > > > My biggest problem with the cover letters is while the are in the
> > > > repository, someplace, I've never actually found one while hunting
> > > > around in the git history for clues, eg with 'git blame' or 'git log
> > > > log -p'
> > >
> > > Well, they don't get merged, this is the point of this thread :-\
> >
> > In the netdev flow, the cover letters are merged.
> > See commit 28f53159e121 ("Merge branch 'vsock-add-flags-field-in-the-vsock-address'")
> 
> But "git log -p -- net/vmw_vsock/af_vsock.c" does not show it.
> You need convoluted commands like Michal posted. And those don't
> take the path directly, so you have to find ${id} and $[branch} first.

Yes exactly, if there was some way to make 'git log -p' or 'git blame'
make the covers visible it would be much more appealing..

Otherwise we merge them into the repo and they are technically
findable but realistically nobody will read them.

Right now if I need to make a merge commit for some reason then I put
the cover letter in there, but needing a merge commit is rare..

Jason

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-01-05 13:38 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 21:32 b4: encouraging using the cover letter in merge commits? Toke Høiland-Jørgensen
2020-12-18 22:09 ` Konstantin Ryabitsev
2020-12-19 12:29   ` Toke Høiland-Jørgensen
2020-12-18 22:38 ` [kernel.org users] " James Bottomley
2020-12-19 12:34   ` Toke Høiland-Jørgensen
2020-12-19 17:03     ` James Bottomley
2020-12-19 17:21       ` Jakub Kicinski
2020-12-19 17:32         ` James Bottomley
2020-12-21 19:05         ` Jason Gunthorpe
2020-12-21 21:13           ` Michal Kubeček
2020-12-21 21:30           ` Arnaldo Carvalho de Melo
2020-12-22  6:30             ` Leon Romanovsky
2020-12-22  8:14               ` Geert Uytterhoeven
2020-12-22 12:36                 ` Leon Romanovsky
2021-01-05 13:38                 ` Jason Gunthorpe
2020-12-19 18:45       ` Jonathan Corbet
2020-12-19 18:49         ` James Bottomley
2020-12-19 18:57           ` Jonathan Corbet
2020-12-19 19:03             ` James Bottomley
2020-12-19 20:48               ` Arnaldo Carvalho de Melo
2020-12-19 21:01                 ` James Bottomley
2020-12-19 21:43                   ` Arnaldo Carvalho de Melo
2020-12-19 21:57                     ` James Bottomley
2020-12-19 22:17                       ` Arnaldo Carvalho de Melo
2020-12-19 23:34                         ` James Bottomley
2020-12-21 17:34       ` [tools] " Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).