Thanks for bringing this topic to our attention Xintong. I think your proposal makes a lot of sense and we should follow it. It will give us confidence that our changes are working and it might be a good incentive to quickly fix build instabilities. Hence, +1.
Cheers, Till On Tue, Jun 22, 2021 at 11:12 AM Xintong Song <tonysong...@gmail.com> wrote: > Hi everyone, > > In the past a couple of weeks, I've observed several times that PRs are > merged without a green light from the CI tests, where failure cases are > considered *unrelated*. This may not always cause problems, but would > increase the chance of breaking our code base. In fact, it has occurred to > me twice in the past few weeks that I had to revert a commit which breaks > the master branch due to this. > > I think it would be nicer to enforce a stricter rule, that no PRs should be > merged without passing CI. > > The problems of merging PRs with "unrelated" test failures are: > - It's not always straightforward to tell whether a test failures are > related or not. > - It prevents subsequent test cases from being executed, which may fail > relating to the PR changes. > > To make things easier for the committers, the following exceptions might be > considered acceptable. > - The PR has passed CI in the contributor's personal workspace. Please post > the link in such cases. > - The CI tests have been triggered multiple times, on the same commit, and > each stage has at least passed for once. Please also comment in such cases. > > If we all agree on this, I'd update the community guidelines for merging > PRs wrt. this proposal. [1] > > Please let me know what do you think. > > Thank you~ > > Xintong Song > > > [1] > https://cwiki.apache.org/confluence/display/FLINK/Merging+Pull+Requests >
