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

[David Chisnall via llvm-dev]

On 25 Jul 2016, at 12:09, Renato Golin <renato.golin at linaro.org> wrote:
> 
> A pull request is a request for *comment*, and we currently express
> that via a Phab review. If I'm comfortable with the patch, I commit
> directly, if I want others opinions, I create a review. If we move as
> you propose, I'd lose the second option.

A pull request is a mechanism.  The community can decide what the policy is.

> I also don't think pre-commit review committers should be just relying
> on the fast bots passing. There are many other issues like code
> quality, formatting, tests, etc. that need to be done properly.

I agree, but it’s much harder to automate that (though having clang-format automatically applied might be nice).

> If we go this route, we'll be effectively flattening out everyone that
> has commit access into one big pool, and that would be a massive
> change in how our community operates (for better or worse).

That’s not what I’m suggesting.  Pull requests provide a convenient staging mechanism.  On the back end, they’re simply git branches in a namespace that won’t be automatically fetched by clients that do a clone.  

> Unless you're also proposing that only the "core team" (code owners,
> high committers, etc) get commit access in the new style.

I’d suggest that no one should have direct push access (or, at least, the no one should *use* the direct access - presumably some people in what GitHub calls the admin groups would retain access for dealing with problems).

>> The workflow would be exactly the same for committers and non-committers, except that committers would be able to tag their own pull requests as ready to merge.
> 
> Can this be controlled via a server hook? One that would be supported by GitHub?

GitHub supports everything that is needed to implement this suggestion.  From the developers’ perspective, the workflow is:

- 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).

In the case of committers who are committing to areas where they are comfortable with post-commit review, these two users can be the same person, so you’d send a pull request and add the tag yourself.

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

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.

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.

Longer term, it would be nice to integrate things like running the static analyser into this workflow.

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.

David

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160725/dc741896/attachment.bin>