[llvm-dev] Notes from GitHub Pull Requests round table

[Thomas Lively via llvm-dev]

The Rust project uses this head-always-green model very successfully. They
have a github bot called bors that manages testing and merging PRs once
they have been approved by code owners. The bot interface is very flexible
and the bot has been extracted into its own project: https://bors.tech/.
You can see it turning through all the approved Rust PRs here:
https://bors.rust-lang.org/queue/rust. The only downside of this system is
that it can take a long time for a PR to be automatically merged since PRs
generally need to be tested one at a time to prevent semantic merge
conflicts. The fix for that is to create "rollups" in which multiple PRs
are tested and landed as a single unit. I'm not sure whether this system
would scale to the commit frequency we have in LLVM, though.

On Thu, Nov 19, 2020 at 2:39 PM Mehdi AMINI via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

>
>
> On Thu, Nov 19, 2020 at 1:56 AM David Chisnall via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On 18/11/2020 17:36, Mike Edwards via llvm-dev wrote:
>> > Hi Keith,
>> > You should be able to access the notes here:
>> >
>> https://docs.google.com/document/d/1flP8TqS71x4KF6h98vZV3ctXADBD39_vjrXn_BQn4hQ/edit?usp=sharing
>> > <
>> https://docs.google.com/document/d/1flP8TqS71x4KF6h98vZV3ctXADBD39_vjrXn_BQn4hQ/edit?usp=sharing
>> >
>> > Let me know if you run into any issues.
>>
>> Thanks.  There's one issue that isn't directly relevant to PRs, but is
>> likely to become more visible if we go to a PR workflow:
>>
>> LLVM takes quite a long time to run the tests and there's usually one
>> commit in between running the test suite on a patch and pushing it to
>> master.  If the result then causes test failures, you won't find out
>> until some time later.  This means I'm generally only willing to hit the
>> merge button near to the start of the day, when I'm likely to be around
>> and paying attention when the failure notices come in, which increases
>> the likelihood that there will be an intermediate patch that causes
>> problems.
>>
>> With a PR-based workflow, it becomes fairly easy to set up a system to
>> manage a queue of patches to be merged, triggered by either a comment
>> from a member of the project or a label (e.g. ready-to-merge) being
>> added to the PR.  The CI infrastructure then rebases the PR on the PR
>> that's ahead of it in the queue (master if it's the front of the queue),
>> kicks off the build, and fast-forwards master when it succeeds.  Head is
>> always in a state where the build bots are happy and if an intermediate
>> change causes problems then that's reflected in the PR that would see
>> the failure, nowhere else.  Subsequent PRs in the queue can rebased on
>> the last passing version and the merge process can continue without a
>> human needing to notice the buildbot failures and revert anything.
>
>
> Such a system is definitely possible, but I wouldn't consider it "fairly
> easy". Do you have examples of OSS infra to achieve this?
> I also don't believe such a system is a guarantee of "always green master
> branch":
> 1) you have flaky test
> 2) it is unlikely that your pre-merge testing can cover the entire matrix
> of configurations supported by the project: all configs (assert/no assert,
> build with shared libraries, etc.) time all sanitizers times all
> subprojects times all host and target platforms?.
>
> (also implementing such a system from scratch does not really seem harder
> conceptually with Phabricator patches vs GitHub PR: fundamentally if a
> phabricator patch can't be applies on top of current HEAD, then the PR
> couldn't be rebased without conflict either for example, ultimately after
> `arc patch` you are manipulating the same git branches locally in your CI).
>
> Thanks,
>
> --
> Mehdi
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201119/19e336d1/attachment.html>