PolymerLabs / project-health

Apache License 2.0
9 stars 6 forks source link

Automerge can merge prematurely #509

Open rictic opened 6 years ago

rictic commented 6 years ago

The Problem

The flow

Expected Behavior

PR is not automerged until tests have completed successfully.

I think what happened is that Travis and Appveyor took a while to add their badges indicating that they were WIP, and maybe the google CLA bot added its badge saying everything was green, so maybe Health saw just that one green badge and thought the PR was good to merge?

Links to Relevant PR's / Issues

This just happened to this PR: https://github.com/Polymer/tools/pull/308

What I think happened

samuelli commented 6 years ago

That's interesting. You may want to consider required status checks (you can pick which you care about) as a workaround which I think will prevent this. Required status checks always expect reporting from CI.

That said, we should be more careful about this, maybe just waiting extra time will be sufficient, but unfortunately it depends on the speed of these services to report as pending. If they're slower than a few seconds, this could be quite problematic.