Re: Please return trailer-order option

[Leon Romanovsky <leon@kernel.org>]

On Fri, Sep 02, 2022 at 10:40:33AM -0400, Konstantin Ryabitsev wrote:
> On Fri, Sep 02, 2022 at 01:25:36PM +0300, Leon Romanovsky wrote:
> > I tried latest b4 with the following config:
> > [b4]
> >          trailer-order = "Cc,Fixes*,Link*,Suggested*,Reviewed*,Tested*,*"
> > 
> > ➜  kernel git:(tmp) ~/src/b4/b4.sh shazam -l -s https://lore.kernel.org/all/20220901202645.1463552-1-irogers@google.com
> > Grabbing thread from lore.kernel.org/all/20220901202645.1463552-1-irogers%40google.com/t.mbox.gz
> > Checking for newer revisions on https://lore.kernel.org/all/
> > Analyzing 2 messages in the thread
> > Checking attestation on all messages, may take a moment...
> > ---
> >   [PATCH v1] selftests/xsk: Avoid use-after-free on ctx
> >     + Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> >     + Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> >   ---
> >   NOTE: install dkimpy for DKIM signature verification
> > ---
> > Total patches: 1
> > ---
> > Applying: selftests/xsk: Avoid use-after-free on ctx
> > 
> > The end result as an outcome of 1 is:
> >     Fixes: 39e940d4abfa ("selftests/xsk: Destroy BPF resources only when ctx refcount drops to 0")
> >     Signed-off-by: Ian Rogers <irogers@google.com>
> >     Link: https://lore.kernel.org/r/20220901202645.1463552-1-irogers@google.com
> >     Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> >     Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > 
> > While I would like to see Link being below Fixes line:
> >     Fixes: 39e940d4abfa ("selftests/xsk: Destroy BPF resources only when ctx refcount drops to 0")
> >     Link: https://lore.kernel.org/r/20220901202645.1463552-1-irogers@google.com
> >     Signed-off-by: Ian Rogers <irogers@google.com>
> >     Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> >     Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> 
> Right, this is the intended change in behaviour -- we don't want to touch
> anything that's above the Signed-off-by that doesn't belong to you. I see why
> you want to move the Link: line above Ian's SoB (to indicate that it's his
> message), but I think from the point of view of chain-of-custody it really
> belongs under it:
> 
>     Fixes: 39e940d4abfa ("selftests/xsk: Destroy BPF resources only when ctx refcount drops to 0")
>     Signed-off-by: Ian Rogers <irogers@google.com>
> 
> This is Ian saying that he's signing off on anything above this trailer. If we
> move Link: above his signoff, this would indicate that he's also signing off
> on the contents of that URL, but that would be putting words in his mouth. If
> we put the Link under it, though, this indicates where *you* got the message, so
> it really belongs in your own custody section:
> 
>     Link: https://lore.kernel.org/r/20220901202645.1463552-1-irogers@google.com
> 
> "I got this message at this URL"
> 
>     Acked-by: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> "I also collected this Acked-by, which can also be found at that URL"
> 
>     Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> 
> "I am signing off on anything above this line"
> 
> Hope this helps explain the reasoning.

Yeah, it explains, the thing is that pure chain-of-custody exists only in
flows that mostly based on pull request.

In my subsystem case, we fix many of submitted patches (spelling,
grammar, coding style and sometimes code itself), because for us the end
goal (having fix/patch merged) is more important than process. We don't want
to scare casual developers with strict rules, of course, the more experienced
submitter, the less tolerant toward his mistakes.

So in my mind, commit message structure is: "Subject, message, Cc, Fixes,
Link, various *-off-by".

Thanks

> 
> -Konstantin