Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[meta] Autorollups #25

Open
1 of 3 tasks
Manishearth opened this issue May 5, 2019 · 14 comments
Open
1 of 3 tasks

[meta] Autorollups #25

Manishearth opened this issue May 5, 2019 · 14 comments

Comments

@Manishearth
Copy link
Member

Manishearth commented May 5, 2019

We should eventually automate rollups, with failures pinging humans.

Steps:

@Manishearth
Copy link
Member Author

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
Copy link
Member Author

cc @Centril

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

@Manishearth
Copy link
Member Author

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
Copy link
Member Author

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.

@Centril
Copy link
Contributor

Centril commented May 5, 2019

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

@Manishearth
Copy link
Member Author

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
Copy link
Member Author

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

@Centril
Copy link
Contributor

Centril commented May 5, 2019

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?

@Manishearth
Copy link
Member Author

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.

@Eh2406
Copy link
Contributor

Eh2406 commented May 5, 2019

Note that I did a lot of work on designing an preparing to make this happen at servo/homu#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.

@Manishearth
Copy link
Member Author

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.

@kennytm
Copy link
Member

kennytm commented May 6, 2019

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

@Manishearth
Copy link
Member Author

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

@RalfJung
Copy link
Member

RalfJung commented Dec 3, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants