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

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Mon, 2022-10-31 at 13:23 -0400, Konstantin Ryabitsev wrote:
> On Mon, Oct 31, 2022 at 01:10:58PM -0400, James Bottomley wrote:
> > > > 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:
> > > 
> > > > 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.
> > 
> > You're way over thinking this.
> 
> Yes, but it's my job to overthink this, so nobody else has to. :)
> 
> > The only tag that matters from the DCO point of view is Signed-off-
> > by. That's the ordering we care about for the chain of
> > custody.  All other tags are irrelevant.  Of course, it's nice to
> > think that reviews happen *after* the code was modified, which is
> > why most of us like to see the Reviewed-by after the initial author
> > signoff,
> 
> But this is where it becomes complicated. The Reviewed-By trailer was
> sent to the v2 of the series, and incorporated into v3(via b4
> trailers). If we stick it below Signed-off-by, then it may suggest
> that Krzysztof reviewed the v4 of the patch.

If the changes from v2 to v4 were material enough for that question to
be relevant then the Reviewed-by tag shouldn't have been kept because
the patch needed reviewing again.

> By placing it above the Signed-off-by line, we at least clearly
> indicate that it's Neil who put it there.

Who put it there does not matter, so it's not really a problem that
needs solving.

However, there is a problem if b4 is preserving Reviewed-by tags for
patches with material changes ... the review has to be redone and thus
the tag should be lost.  You can only keep Reviewed-by tags for
cosmetic changes (or obviously patches which don't change from version
to version).

James