Re: DCO chain of custody revisited (was Re: [PATCH v4 08/11] mfd: qcom-pm8xxx: drop unused PM8018 compatible)

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Oct 31, 2022 at 12:58:42PM -0400, Konstantin Ryabitsev wrote:
> Hijacking this thread for greater good.
> 
> On Mon, Oct 31, 2022 at 04:35:38PM +0100, Neil Armstrong wrote:
> > Hi,
> > 
> > On 31/10/2022 16:32, Lee Jones wrote:
> > > On Fri, 21 Oct 2022, Neil Armstrong wrote:
> > > 
> > > > The PM8018 compatible is always used with PM8921 fallback, so PM8018
> > > > compatible can be safely removed from device ID table
> > > > 
> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > 
> > > Tags should appear chronologically.
> > 
> > Indeed, they were added by b4, I'll report this.
> 
> My trouble is that there are seemingly as many opinions about the order of
> trailers as there are subsystem maintainers. The last time we had a long
> discussion about this on the users list I got a strong message that what
> matters most is the chain of custody, and the Signed-off-by trailer indicates
> the chain of custody boundary.
> 
> In the scenario below, the chain consists of 3 people:
> 
> | Suggested-by: Reporter 1 <...>
> | Signed-off-by: Developer 1 <...> -- initial DCO boundary
> | Reviewed-by: Reviewer 1 <...>
> | Tested-by: Tester 1 <...>
> | Signed-off-by: Submaintainer 1 <...> -- intermediate DCO boundary
> | Acked-by: Submaintainer 2 <...>
> | Signed-off-by: Maintainer 1 <...> -- final DCO boundary
> 
> In terms of DCO, this makes the following claims:
> 
> Developer 1:
>  - I am responsible for this change
>  - It was suggested by Reporter 1
> 
> Submaintainer 1:
>  - I am signing off on this change
>  - I have collected the trailers from Reviewer 1 and Tester 1
> 
> Maintainer 1:
>  - I am signing off on this change
>  - I have collected the trailer from Submaintainer 2
> 
> In the current case, and using this principle, the following order is correct:
> 
> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> Because the Reviewerd-by trailer was sent to the v2 of the series and was
> collected by Neil, so Neil is the person who is the DCO signatory of that
> chain of custody in the v4 of the series.
> 
> I assume that in the final commit Lee rearranged the tags in the following
> order:
> 
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Lee Jones <lee@kernel.org>
> 
> This would indicate that it's *Lee* who is claiming responsibility for
> collecting the Reviewed-by tag from Krzysztof, because it is in his chain of
> custody. However, this is not the case -- it was Neil who collected the tag,
> and therefore the "more correct" order should be:


I think this is often incorrect:
I often pick a patch, then amend it adding tags from reviewers,
typically I add tags at the end just because it's easier,
and tools like git-interpret-trailers exist.

> | Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> | Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> | Signed-off-by: Lee Jones <lee@kernel.org>
> 
> If my reasoning is incorrect, then I need to go back to the drawing board.
> 
> -K