One way to address the problem of regularly failing tests that block merging of PRs is to disable the respective tests for the time being. Of course, the failing test then needs to be fixed. But at least that way we would not block everyone from making progress.
Cheers, Till On Tue, Jun 22, 2021 at 12:00 PM Arvid Heise <ar...@apache.org> wrote: > I think this is overall a good idea. So +1 from my side. > However, I'd like to put a higher priority on infrastructure then, in > particular docker image/artifact caches. > > On Tue, Jun 22, 2021 at 11:50 AM Till Rohrmann <trohrm...@apache.org> > wrote: > > > 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 > > > > > >
