[PR] stability

[honstar]

Hi folks!

I've been looking into milo PRs lately and noticed that the majority of them are failing one or more checks. While I'm not questioning the quality of the code being contributed at all, it makes it fairly difficult to review and eventually approve and merge PRs.

Do we know why that is? Is it possible that some thresholds have been set too aggressively?

[hparra]

See #621 for part of the solution

[hparra]

@honstar I agree that a green check on a PR should signal "i'm ready to be reviewed"

Issues that remain:

• codecov is flaky
  • See unexpected coverage changes
  • See Improve Codecov config for unexpected coverage changes #621 for fix
• unit tests are flaky
  • mitigation: rerun tests
  • we really should fix them
• psi sometimes bad for real-world examples (e.g. video embed)
  • maybe change so only LCP is used?
• PR needs either verified or trivial
  • there should be a needs-verification so that build passes but still communcates e2e check needed
  • we can change settings to block merge if does not have verified or trivial

For each one we fix properly we should then configure rule to block merge on failure.

CC: @auniverseaway

[hparra]

FYI, #621 itself, despite being only one change (a config file) demonstrated some of these issues:

• codecov report says my coverage increased (???)
• i had rerun unit tests twice to get it to pass

[auniverseaway]

Here's my thoughts:

1. If codecov is flaky, let's fix.
2. if unit tests are flaky, let's fix.
3. We won't be changing PSI. I think this is a developer education thing.
4. Agreed on the PR thing... shouldn't cause a red X. That whole label action needs to die and we should write our own.

[honstar]

Since we agree overall to fix flaky unit-tests, we should prioritize the work involved. We already have a(n internal) JIRA issue logged.

[meganthecoder]

@auniverseaway I'm working on label actions for bacom and I'm curious what you mean for #4. "That whole label action needs to die and we should write our own." What do you think the new action should do?