Skip to contentSkip to conversation

b4: encouraging using the cover letter in merge commits?

DISCUSSION Toke Høiland-Jørgensen 11 participants 26 messages Dec 18, 2020 – Jan 5, 2021

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 4 lines · Toke Høiland-Jørgensen

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

#2 lore raw
Quoted 11 lines · Konstantin Ryabitsev
Quoted 4 lines · Toke Høiland-Jørgensen

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 :)

Quoted 10 lines

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

#3 lore raw
In reply to Toke Høiland-Jørgensen
Quoted 7 lines · Toke Høiland-Jørgensen via linux.kernel.org

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?

Quoted 5 lines

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

#4 lore raw
Quoted 22 lines · James Bottomley
Quoted 7 lines · Toke Høiland-Jørgensen via linux.kernel.org

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?

Quoted 5 lines

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?

Quoted 5 lines

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

#5 lore raw
Quoted 35 lines · Toke Høiland-Jørgensen via linux.kernel.org
Quoted 22 lines · James Bottomley
Quoted 7 lines · Toke Høiland-Jørgensen via linux.kernel.org

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?

Quoted 5 lines

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?

Quoted 5 lines

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:

Quoted 5 lines

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

#6 lore raw
Quoted 7 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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/

#7 lore raw
Quoted 23 lines · Jakub Kicinski
Quoted 16 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 8 lines

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:

Quoted 7 lines

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 :)

Quoted 5 lines

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?

Quoted 8 lines

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.

Quoted 3 lines

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

Quoted 4 lines

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/

#8 lore raw
In reply to Jakub Kicinski
Quoted 26 lines · Jakub Kicinski
Quoted 7 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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

#9 lore raw
Quoted 5 lines · Jason Gunthorpe

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'

Quoted 7 lines

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

#10 lore raw
In reply to Jason Gunthorpe

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:

Quoted 27 lines · Jakub Kicinski
Quoted 26 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 3 lines

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 :-)

Quoted 7 lines

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

#11 lore raw
Quoted 32 lines · Arnaldo Carvalho de Melo

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:

Quoted 30 lines · Jakub Kicinski
Quoted 29 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 4 lines

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

#12 lore raw

Hi Leon,

Quoted 38 lines · Leon Romanovsky
Quoted 37 lines · Arnaldo Carvalho de Melo

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:

Quoted 35 lines · Jakub Kicinski
Quoted 26 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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'

Quoted 4 lines
Quoted 1 line

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

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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
#13 lore raw
Quoted 41 lines · Geert Uytterhoeven

Hi Leon,

Quoted 38 lines · Leon Romanovsky
Quoted 37 lines · Arnaldo Carvalho de Melo

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:

Quoted 35 lines · Jakub Kicinski
Quoted 26 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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'

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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

Quoted 11 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

#14 lore raw
In reply to Geert Uytterhoeven
Quoted 41 lines · Geert Uytterhoeven

Hi Leon,

Quoted 38 lines · Leon Romanovsky
Quoted 37 lines · Arnaldo Carvalho de Melo

Em Mon, Dec 21, 2020 at 03:05:52PM -0400, Jason Gunthorpe escreveu:

Quoted 35 lines · Jakub Kicinski
Quoted 26 lines · On Sat, 19 Dec 2020 09:03:36 -0800 James Bottomley wrote:
Quoted 7 lines
Quoted 7 lines

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

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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'

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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

#15 lore raw
In reply to James Bottomley
Quoted 4 lines · On Sat, 19 Dec 2020 09:03:36 -0800 "James Bottomley"…

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

#16 lore raw
Quoted 8 lines · Jonathan Corbet
Quoted 4 lines · On Sat, 19 Dec 2020 09:03:36 -0800 "James Bottomley"…

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?

Quoted 5 lines

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

#17 lore raw
Quoted 2 lines · On Sat, 19 Dec 2020 10:49:21 -0800 "James Bottomley"…

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

#18 lore raw
Quoted 10 lines · Jonathan Corbet
Quoted 2 lines · On Sat, 19 Dec 2020 10:49:21 -0800 "James Bottomley"…

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.

Quoted 4 lines

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

#19 lore raw

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:

Quoted 22 lines · Jonathan Corbet
Quoted 10 lines
Quoted 2 lines · On Sat, 19 Dec 2020 10:49:21 -0800 "James Bottomley"…

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.

Quoted 4 lines

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.

Quoted 5 lines

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

#20 lore raw
Quoted 2 lines · Arnaldo Carvalho de Melo

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:

Quoted 1 line · Jonathan Corbet

[...]

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

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.

Quoted 8 lines
Quoted 6 lines

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.

Quoted 1 line

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.

Quoted 2 lines

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

#21 lore raw

Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:

Quoted 16 lines · Arnaldo Carvalho de Melo
Quoted 2 lines

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:

Quoted 1 line · Jonathan Corbet

[...]

Quoted 12 lines
Quoted 11 lines
Quoted 4 lines

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.

Quoted 4 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 7 lines
Quoted 2 lines

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.

Quoted 2 lines

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

Quoted 9 lines

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

Signature · 1 line
- Arnaldo
#22 lore raw
Quoted 26 lines · Arnaldo Carvalho de Melo

Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:

Quoted 18 lines · Arnaldo Carvalho de Melo
Quoted 3 lines

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:

Quoted 1 line · Jonathan Corbet

[...]

Quoted 13 lines
Quoted 12 lines
Quoted 5 lines

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.

Quoted 4 lines
Quoted 1 line

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.

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

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.

Quoted 10 lines
Quoted 7 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 3 lines

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

#23 lore raw

Em Sat, Dec 19, 2020 at 01:57:06PM -0800, James Bottomley escreveu:

Quoted 33 lines · Arnaldo Carvalho de Melo
Quoted 32 lines

Em Sat, Dec 19, 2020 at 01:01:43PM -0800, James Bottomley escreveu:

Quoted 18 lines · Arnaldo Carvalho de Melo
Quoted 3 lines

Em Sat, Dec 19, 2020 at 11:03:59AM -0800, James Bottomley escreveu:

Quoted 1 line · Jonathan Corbet

[...]

Quoted 13 lines
Quoted 12 lines
Quoted 5 lines

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.

Quoted 4 lines
Quoted 1 line

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.

Quoted 5 lines
Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 5 lines

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.

Quoted 14 lines
Quoted 13 lines
Quoted 7 lines
Quoted 2 lines

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.

Quoted 3 lines

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

Quoted 6 lines
Quoted 4 lines

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...

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 6 lines
Quoted 3 lines

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

#24 lore raw
Quoted 6 lines · Arnaldo Carvalho de Melo

Em Sat, Dec 19, 2020 at 01:57:06PM -0800, James Bottomley escreveu:

Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 3 lines

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?

Quoted 8 lines
Quoted 2 lines

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.

Quoted 2 lines

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

#25 lore raw
In reply to James Bottomley
Quoted 4 lines · James Bottomley

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.