Hi Kurt,

Thanks for your reply!

I find two concerns about the downside from your email. Correct
me if I misunderstanding.

1. Rebase times. Typically commits are independent one another, rebase
just fast-forward changes so that contributors rarely resolve conflicts
by himself. Reviews doesn't get blocked by this force rebase if there is
a green travis report ever -- just require contributor rebase and test
again, which generally doesn't involve changes(unless resolve conflicts).
Contributor rebases his pull request when he has spare time or is required
by reviewer/before getting merged. This should not inflict too much works.

2. Testing time. It is a separated topic that discussed in this thread[1].
I don't think we finally live with a long testing time, so it won't be a
problem then we trigger multiple tests.

Simply sum up, for trivial cases, works are trivial and it
prevents accidentally
failures; for complicated cases, it already requires rebase and fully tests.

Best,
tison.

[1]
https://lists.apache.org/x/thread.html/b90aa518fcabce94f8e1de4132f46120fae613db6e95a2705f1bd1ea@%3Cdev.flink.apache.org%3E


Kurt Young <ykt...@gmail.com> 于2019年8月30日周五 上午9:15写道:

> Hi Zili,
>
> Thanks for the proposal, I had similar confusion in the past with your
> point #2.
> Force rebase to master before merging can solve some problems, but it also
> introduces new problem. Given the CI testing time is quite long (couple of
> hours)
> now, it's highly possible that before your test which triggered by rebasing
> finishes,
> the master will get some more new commits. This situation will get worse if
> more
> people are doing this. One possible solution is let the committer decide
> what should
> do before he/she merges it. If it's a trivial issue, just merge it if
> travis passes is
> fine. But if it's a rather big one, and some related codes just got merged
> in to master,
> I will choose to rebase to master and push it to my own repo to trigger my
> personal
> CI test on it because this can guarantee the testing time.
>
> To summarize: I think this should be decided by the committer who is
> merging the PR,
> but not be forced.
>
> Best,
> Kurt
>
>
> On Thu, Aug 29, 2019 at 11:07 PM Zili Chen <wander4...@gmail.com> wrote:
>
> > Hi devs,
> >
> > GitHub provides a mechanism which is able to require branches to be
> > up to date before merged[1](point 6). I can see several advantages
> > enabling it. Thus propose our project to turn on this switch. Below are
> > my concerns. Looking forward to your insights.
> >
> > 1. Avoid CI failures in pr which fixed by another commit. We now merge a
> > pull request even if CI fails but the failures knowns as flaky tests.
> > We doesn't resolve this by turn on the switch but it helps to find any
> > other potential valid failures.
> >
> > 2. Avoid CI failures in master after pull request merged. Actually, CI
> > tests the branch that pull request bind exactly. Even if it gave green
> > it is still possible a systematic failure introduced because conflicts
> > with another new commit merged in master but not merged in this branch.
> >
> > For the downside, it might require contributors rebase his pull requests
> > some times before getting merged. But it should not inflict too much
> > works.
> >
> > Best,
> > tison.
> >
> > [1] https://help.github.com/en/articles/enabling-required-status-checks
> >
>

Reply via email to