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

[Linus Arver <linusa@google.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

> 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?

I don't have much history on this list to know one way or the other, but
it would certainly help to double-check all of the advice contained in
here for accuracy. 

I also think that we need to add some more structure to the
SubmittingPatches doc. It is currently pretty long and could use some
help in being broken up a bit more. 

One thing I noticed while drafting this series was that we don't really
separate minutiae from what is _really_ important. For example even the
advice around adding "Acked-by:" and other trailers --- is it really
critical? Other than the "Signed-off-by: " of the patch author (required
for legal reasons), it's not the end of the world if someone forgot to
add a "Reviewed-by: ". We should do a better job of separating
absolutely critical things that must be done correctly to ensure smooth
function of the review process, from the rest that are not so important.