Glow: Make sure internal Facebook PRs pass CircleCI before merge

[tlepley-cadence]

Lately, it's getting difficult to contribute to Glow. Working PRs fail in the CI due to a problems with the master branch, then their landing gets delayed, and contributors need to spend more time rebasing, retesting.

Lately, problems I've seen are:

• issue with pytorch (was fixed by disabling the related CI test)
• code formatting issue (was fixed few days ago)
• now, it seems the new 'folly' dependency makes the CI fail

I'm just wondering how PRs that break get merged while they break the CI.
My assumption is that no PR can't land without first being tested by the CI .

Am I missing something ?

@jfix71 @opti-mix @ayermolo

[jfix71]

Hi @tlepley-cadence, apologies for the frustration, we feel it as well. The problem here is that our internal CI provides different coverage than CircleCI does, and the internal system only blocks merging based on the internal CI, and not based on CircleCI. I can try looking into making CircleCI blocking, or at least more visible to the author to try to prevent future breakages. And we could do a better job of unbreaking things more quickly as well.

[tlepley-cadence]

@jfix71 yes, I agree that code merge should be only allowed if CircleCI also passes. Otherwise internal development at Facebook will continuously break the github project, and this is what has happened lately. I'll then rename this issue to reflect this target.

[bertmaher]

now, it seems the new 'folly' dependency makes the CI fail

I don't think folly actually broke anything on CI, did it? From the logs it seemed like another diff that landed around the same time broke the coverage build.

[bertmaher]

Unfortunately, FB internal CI isn't set up to block on CircleCI. I did take a pass through our CircleCI builds to remove some low-value builds that had a tendency to fail for non-fundamental reasons (SHARED and COVERAGE). The others should generally be in sync, so the likelihood of breakage is lower.