* 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: 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 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: [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: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
* 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
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).