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