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

[Mehdi AMINI via llvm-dev]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201119/92e77381/attachment.html>