Re: [RFC] How to accellerate the patch flow (or should we?)

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

>> ... (note that this is based on the assumption
>> that "find any remaining bugs while it is in 'next' before it hits
>> 'master'" philosophy is working, but we have never run experiments
>> to shorten this to say 3 days to see if we see more bugs on 'master'
>> yet).
> ...
> I have a vague recollection that Google internally has their engineers
> run a version of Git that is based on 'next'. But after spending a few
> minutes searching through the list archives, I can't seem to find any
> record of that.

They do, but the frequency they update desktop installations is lower
than the frequency I merge new topics to update the tip of 'next', so
I suspect they alone would not be sufficient guinea pigs.

> Maybe tooling could help with this. But I think that we could also push
> ourselves to be more proactive through policy changes. What if we
> required that (unless the maintainer wants to unilaterally merge a
> topic) that at least one other reviewer must give it a positive "ack"
> before the maintainer starts merging it down?
> ...
> The best that I can come up with is the above (one or more contributors
> must provide a positive "ack" before the maintainer merges a topic, up
> to the maintainer's discretion). I think something like the following
> would be worth trying:
>
> --- 8< ---
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 86ca7f6a78a..789febefff8 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -506,7 +506,10 @@ 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.
> +instead left the review to trusted others.  Patch series must receive
> +a positive "ack" from at least one contributor other than the primary
> +patch series author in order to begin integrating it, subject to the
> +maintainer's discretion.
>
>  Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
>  `Tested-by:` lines as necessary to credit people who helped your
> --- >8 ---

It would lead us into ugly awkwardness when we start clarifying what
exactly "contributor" is in the new sentence, though.  If a person,
whom none of us have ever heard of, sends their first message to
this list saying "Ack", does that count?  If an active developer,
who is known to be sloppier than others, sends an "Ack" to somebody
else's patch that was posted 3 hours before (hence there wouldn't
have sufficient time to think through the issues), how much should
that "Ack" weigh?

Perhaps rephrasing it to "those who have helped in polishing the
patches with their reviews and discussing the issues with the patch
author" to tighten the language a bit may help?

I dunno, as that would still give the "ack right" to a random
noisemaker who threw a drive-by "review" that did not add much value
to the patches, if the original author responded "Thanks" out of
courtesy.

> I am not sure the idea of adding more maintainers is a good one or not.
> Since I am not sure exactly what you are envisioning here, I think there
> are a couple of cases:
>
>  - There is a quorum of maintainers, who are all collectively
>    responsible for building 'jch', 'seen', 'next', and 'master'. Any two
>    of them are required to agree to move a topic down in order to do so,
>    without needing the other maintainer to weigh in.
>
>  - There are sub-system maintainers who are responsible for their own
>    trees, and who send pull requests to the primary maintainer to
>    integrate their trees back into the primary maintainer's.

Git is not large enough to benefit from the latter arrangement.
What I had in mind was the former one.

> The "quorum of maintainers" idea has a couple of drawbacks. Unless one
> maintainer has the special role of being able to unilaterally merge
> topics, each topic would require twice as many maintainers as we
> currently have in order to merge it down ;-). I could also imagine there
> being some coordination overhead between maintainers when shuffling who
> is building the integration branches to make sure that topics don't
> mysteriously get added or dropped when shuffling between maintainers.

True.  It was a strawman to invite other more realistic ideas (like
your "positive ack required"), and was not necessarily designed to
be workable ;-).