Re: [PATCH] checkpatch: validate commit tag ordering

[Konstantin Ryabitsev <konstantin@linuxfoundation.org>]

On Thu, Jul 24, 2025 at 09:20:32AM +0200, Hendrik Hamerlinck wrote:
> Modified the checkpatch script to ensure that commit tags (e.g.,
> Signed-off-by, Reviewed-by, Acked-by, Tested-by, etc.) appear in the
> correct order according to kernel conventions [1].

As already indicated, this is the convention used by the TIP tree and is not
universal. Moreover, there is a lot more nuance to how trailers are used, with
many other subsystems strongly preferring chain-of-custody boundaries. For
example, the following trailers indicate a history of how the patch was
reviewed:

| Suggested-by: Sug Gester <sug@example.com>
| Signed-off-by: Alex Dev <alex@example.com>          -- boundary 1
| Acked-by: Acker Mack <acker@example.com>
| Tested-by: Test Rogen <test@example.com>
| Signed-off-by: Sub Maintainer <sub@example.com>     -- boundary 2
| Reviewed-by: Rev Yewer <rev@example.com>
| Tested-by: Integration Bot <int@example.com>
| Link: https://patch.msgid.link/foomsgid@exmple.com
| Signed-off-by: Main Tainer <main@example.com>       -- boundary 3

There are three chain of custody boundaries in the example above and in the
chain-of-custody scenario the trailers should NOT be moved around between
these boundaries, because each of the boundaries indicates what each
signed-off-by person is claiming as their responsibility.

Everything above boundary 1 is claimed by Alex Dev; all trailers above
boundary 2 were collected and applied by Sub Maintainer; all trailers
above boundary 3 were collected and applied by Main Tainer. Alex Dev has no
responsibility for the tag provided by the Integration Bot, so moving their
signed-off-by to the bottom of this series of trailer would imply that they
are.

I would leave the trailer order entirely alone and out of tools like
checkpatch, so this is a gentle but firm NACK from me.

-K