homu icon indicating copy to clipboard operation
homu copied to clipboard

[meta] Autorollups

Open Manishearth opened this issue 5 years ago • 14 comments

We should eventually automate rollups, with failures pinging humans.

Steps:

  • [x] Add rollup levels (https://github.com/rust-lang/homu/issues/22)
  • [ ] Add travis status indicators to the list view (https://github.com/rust-lang/homu/issues/26)
  • [ ] Add a subsystem that picks a bag of approved non-never-rollup PRs, balancing travis-pass rollup=always and rollup=maybe counts based on some heuristics

Manishearth avatar May 05 '19 19:05 Manishearth

I'd long ago designed a more elaborate version of this, but that requires a ton more infra (and funding!) to make work.

This system works with what we've got.

Manishearth avatar May 05 '19 19:05 Manishearth

cc @centril

been meaning to file this for ages, but keep forgetting. #22 reminded me that we could have such a thing

Manishearth avatar May 05 '19 19:05 Manishearth

Also we don't have to actually add an automatic subsystem like the one at the bottom; we can still have rollup initiation be manual but by default based on these heuristics, making it something we can ask more people to do.

Manishearth avatar May 05 '19 19:05 Manishearth

We also shound write up policies for rollup=foo markers and share them with reviewers, namely:

  • build system stuff should usually be never
  • non-code changes can be always, as well as anything else you are confident will not break. We can extend this to "confident will not break given a default travis run passes".
  • If a PR breaks a rollup, the default thing to do should be to mark it as rollup=never, unless you're very confident that it's fixed.

Manishearth avatar May 05 '19 19:05 Manishearth

More guidelines:

  • "build system stuff" includes (not full list) changes to bootstrap, build-manifest, and compiletest
  • If you change .lock files, do not rollup
  • If a PR is perf sensitive, mark it as never
  • If a PR is quite large and isn't just adding a bunch of files (e.g. tests) then it should be never

Heuristics:

  • As for heuristics, we should have some (configurable) limit on how many PRs go into an auto-rollup.
  • If a PR is prioritized and isn't never, it should go into a rollup before lower prio PRs.
  • Do/should we have heuristics re. conflicts (in favor of fewer)?

Centril avatar May 05 '19 20:05 Centril

As for heuristics, we should have some (configurable) limit on how many PRs go into an auto-rollup.

Yeah I'm imagining a total cap on # of PRs, and a cap on # of maybe PRs.

Manishearth avatar May 05 '19 20:05 Manishearth

Bors already excludes conflicting PRs from rollups, so that's fine.

Manishearth avatar May 05 '19 20:05 Manishearth

Bors already excludes conflicting PRs from rollups, so that's fine.

Hmm; but how does it choose which PR to remove? E.g. say we have 1 big PR and 4 smaller PRs and all of the 4 PRs conflict with the 1 big PR, does it pick the big one or the small ones?

Centril avatar May 05 '19 20:05 Centril

I think it's by whatever sort order the client submits it in. As in, it keeps trying to merge PRs in the order it's asked to, and then excludes any that it can't.

We could add an explicit internal sort but I don't think this is too valuable.

Manishearth avatar May 05 '19 20:05 Manishearth

Note that I did a lot of work on designing an preparing to make this happen at https://github.com/servo/homu/issues/102. All sow the developers behind bors-ng have reached out in the past to find out what changes they could make so that rust could switch to bors-ng. https://internals.rust-lang.org/t/proposal-move-to-bors-ng/5334

The answer to both attempt, from Mark_Simulacrum and brson, was that Autorollups would not be acceptable for rust.

Opinions, people, or reality may have changed since then.

Eh2406 avatar May 05 '19 20:05 Eh2406

Most of the steps here are useful to rollup authors even without the rollups being 100% automatic. An "automatically create a rollup" button would go a long way.

Furthermore, we're doing daily rollups now, so stuff has changed.

I think it will be easier to add support to homu than to get bors-ng to support everything we need here, though.

Manishearth avatar May 05 '19 21:05 Manishearth

I'm opposed to adding any sophisticated changes like automated rollup when homu doesn't even have proper testing (the CI just runs a style check).

kennytm avatar May 06 '19 13:05 kennytm

Cool. In that case, I think we should still add everything up to the automatic rollup: having travis checks, and a "i'm feeling lucky"-style rollup button that does all the choosing for you (maybe client-side).

Manishearth avatar May 06 '19 15:05 Manishearth

Related work: https://github.com/bors-ng/bors-ng seems to do something like auto roll-ups with its concept of "batches".

RalfJung avatar Dec 03 '19 08:12 RalfJung