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