Re: [PATCH v3 5/8] SubmittingPatches: discuss reviewers first

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Apr 9, 2024 at 5:57 PM Linus Arver via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> No matter how well someone configures their email tooling, understanding
> who to send the patches to is something that must always be considered.
> So discuss it first instead of at the end.
>
> In the following commit we will clean up the (now redundant) discussion
> about sending security patches to the Git Security mailing list.
>
> Signed-off-by: Linus Arver <linusa@google.com>
> ---
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> @@ -397,6 +397,36 @@ letter.
> +After the list reached a consensus that it is a good idea to apply the
> +patch, re-send it with "To:" set to the maintainer{current-maintainer}
> +and "cc:" the list{git-ml} for inclusion.  This is especially relevant
> +when the maintainer did not heavily participate in the discussion and
> +instead left the review to trusted others.

This isn't a new problem since you're merely relocating this text
(thus, very likely may be outside the scope of this series), but is
this recommendation still accurate? Although I'm unable to locate it
in the mailing list, I have some vague recollection of Junio
relatively recently wondering why a patch series had been resent with
no changes and why he had been made the direct To: recipient. It
turned out that the author was following the above instructions.

Generally speaking, Junio is quite good at picking up a patch series
without the author having to follow these instructions to resend a
patch series with no changes other than the To: header, so such
instructions place unnecessary burden upon both submitters as well as
reviewers (who have to spend extra cycles wondering why a series was
rerolled and whether any changes were made).

It would probably be more helpful (and less wasteful of reviewer time)
to instruct the patch submitter to monitor "What's Cooking" and
Junio's "seen" branch, and to ping the list (after a week or two) if
the patch series hasn't been picked up or seen any response.

> +Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
> +`Tested-by:` lines as necessary to credit people who helped your
> +patch, and "cc:" them when sending such a final version for inclusion.

Again, not a new problem introduced by this patch, but it seems like
all of these are actively wrong. In every case, these trailers are
_given_ by reviewers _after_ a series has been submitted (thus, too
late for the author to add them), and Junio typically is the one who
latches the Reviewed-by:, Acked-by:, etc. by adding the trailer to the
patches already in his tree.

Instead of the above, much more useful trailers that a patch author
can add are Helped-by: and Reported-by:.