Re: [PATCH v2] Writing down mail list etiquette.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 12, 2021, 14:36 UTC
- Message-ID
- <xmqqbl9gf299.fsf@gitster.g>
- In-Reply-To
- <CAPig+cQvnsRe0fdPaBe9hJ4LbPFmHmGVNdiGYLqi2JM7A5GmjA@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> This has been my experience, as well. I've never sent a v6 with Junio > as an explicit recipient, but which was otherwise identical to v5.
It mostly is because I've been too helpful than the written rule to proactively pick up v5, before the participants of the discussion explicitly reaches concensus that it is good enough.
In an ideal world, patches in areas that I do not have particular interest in and that other reviewers with good taste are performing good reviews, I should be able to keep myself completely out of the picture, not involved in their review discussion in any way, and not even having to monitor if the discussion reached some concensus, and purely play a patch-monkey for such patches. That, combined with more reviewers with good tastes, would allow us to injest patches at faster rate than we currently can.
> Another reason to avoid sending v6 which is identical to v5 is that it > potentially wastes reviewer bandwidth.
As long as it is marked as "this is the final version that is identical to the previous round, only difference from which is that the collected reviewed-by and acked-by have been added", it would not waste reviewer bandwidth. The reviewers _should_ however notice if it has questionable changes since the "previous round", in which case their reviewed-by's may now be invalid, of course, though.
Show 16 quoted lines
> The advice which seems to have triggered this particular discussion
> comes from Documentation/SubmittingPatches:
>
> 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.
>
> It's not the first time this advice has resulted in confusion. Perhaps
> now would a good time to retire it altogether, or at least rewrite it
> to mention the points you gave above about responding to "What's
> Cooking" or by sending a "ping" to the original patch email (which may
> result in Junio either picking up the patch or responding with an
> explanation as to why he didn't).No, please do not put _more_ tasks on my plate.
The "send the final with everybody's concensus" is an ideal that tries to reduce the maintainer bottleneck by distributing the work of final validation. I've been too helpful in this area in that I have often been the one who does the "ping" ("What is the status of this topic?"), but that would _not_ scale. Anything that the party with more numbers, namely the contributors, can do, we should farm it out to them.
Show 12 quoted lines
> Following the above SubmittingPatches paragraph is another which also > seems to mislead people frequently: > > 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. > > In fact, a submitter should almost never add a Reviewed-by: trailer > because Reviewed-by: is explicitly given by a reviewer only when the > reviewer is satisfied that the patch is merge-ready, in which case no > more re-rolls are expected.
Yes, that is exactly why v6 that is identical to v5 that all reviewers are happy with is useful. It should be able to carry reviewed-by's and acked-by's, reviewers can audit that there is no forged or otherwise inappropriate reviewed-by's, without placing any extra burden on the maintainer.
Thanks.