Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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.
> 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.
> 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.
