[llvm-dev] [RFC] One or many git repositories?

[Renato Golin via llvm-dev]

On 25 July 2016 at 12:40, David Chisnall <david.chisnall at cl.cam.ac.uk> wrote:
> I’d suggest that no one should have direct push access

That's what I meant by "massive change". :)

I don't think that's a bad model, but it's a very different model than
we currently have, so would need more discussion on that side before
we can even think about the technical details.


> - User sends pull request.
> - User from a list of people that correspond to our current set of committers labels the request as ‘approved’ (GitHub labels can only be applied by members of the organisation).
> The buildbot master then:
> - Collects all of the pull requests (git fetch)
> - Uses the GitHub API to filter the ones that have been approved
> - Merges these, in turn, to its local master branch
> - Sends those changes to the builders
> - If they succeed, pushes master to GitHub

This sounds a lot like the Gerrit model.

Can buildbots do that? How much code would be needed?

Wouldn't it be easier to do this in Jenkins (push based)?


> In idle time (when not busy with approved patches), it should also try other pull requests and add a label saying that they’ve passed the tests.

This sounds like an added complexity to the buildbot code. (priority
queues?) Not impossible, maybe not even hard, but yet another small
thing to get right...


> There are some obvious refinements to this, for example only kicking the fast buildbots when there are several patches outstanding and then running the slower ones with the combined master, then backtracking and applying them one at a time if this fails.

If this is going to work, we'll need to run a single pull request on
top of the correct "master" for every change.

IIUC, GitHub will "trigger" a CI change for every *update* on every
pull request, too. So we'll end up with a lot more half-baked patches
than usually go through the current buildbots.

This will mean more builders per bot (I'm expecting at least 3) and a
silent bots (we can use 8014 for that).


> I don’t have particularly strong feelings about this, I just wanted to point out that it’s possible with GitHub and a little bit of client code talking to the GitHub APIs.  There’s a slightly more complex workflow for people to push code, but in exchange for that we’d never be in a state where anything pushed to master introduces regressions for which tests exist.

I'm a big advocate of pre-commit tests and the Gerrit model, I think
it makes developers more aware of their own failures (which bbots do)
without upsetting the rest of the community (which bbots don't).

For instance, in this model, I only need to review code that has been
marked by the pre-commit bots as "good", so it also saves a lot of
time from code reviewers, in addition to buildbot owners, and it will
make the releases, back-porting, bisecting, *much* easier by the
massive reduction in silly reverts.

If I understand correctly, the model you propose is:

1. Admins: people with direct commit access used only for fixing the
repo, fork releases, etc.
2. Pre-review committers: people with the right to create a
"auto-merge" tag on pull reviews, but not commit directly.
3. Post-review committers: just pull request access, need to wait for
someone in (2) to approve
4. Everyone else: just pull request access, need to wait for someone
in (2) to approve

A few problems with this:

A. How do we divide the current (fuzzy) "groups" into those well defined groups?
B. (2) folks will have the choice to get reviews, (3) won't have the
choice to quickly fix their commits (which some do today).
C. The migration between (3) to (2) is not defined *at all* today, and
this is where the most radical cut will be.

Post-review commit access is, as I understand, a social recognition
for those striving to become more active committers in the future. By
flattening (3) and (4), any threshold we pick will be "wrong" in some
form, and may discourage people from contributing changes, or striving
to become pre-review committers.

I'm not saying you're wrong, or that I don't agree with you, but this
won't be an easy sell...

cheers,
--renato