rust-lang / triagebot

Automation/tooling for Rust spaces
https://triage.rust-lang.org
Apache License 2.0
170 stars 74 forks source link

Post Rollup Warnings #1657

Open rylev opened 1 year ago

rylev commented 1 year ago

When creating rollups some PRs are naturally riskier than others. If the rollup introduces bugs or performance issues it can be helpful to get a hint at what PRs might be causing an issue and which PRs can most likely be ignored.

With this in mind, I propose adding support to triagebot for listening for rollup PRs and posting a message with the results of inspecting how risking the rollup is. It could use simple heuristics to begin with.

Configuration Changes

The following would be added to the triagebot.toml:

[rollup-warnings]
trigger_files = [
  "compiler/**/*.rs" # warn on any Rust files in the compiler directory
  # ... more patterns here
]

Warning

For any rolled-up PR included in a rollup that has a file change identified by the patterns in trigger_files, a warning would be posted that looks like the following:

⚠Vulnerable PRs

This rollup has 2 vulnerable PRs.

The following PRs may be particularly vulnerable to breakage:

If this rollup introduces any type of regression, you may wish to investigate these PRs first.

Trigger Rate

Looking at some recent rollups, I expect this warning to show the following results:

Future extensions and open questions

jyn514 commented 1 year ago
  • Some changes are likely a bad idea to have in a rollup. For example, in rust-lang/rust, a PR that changes over 50 files in compiler should probably not have been rolled up (or at the very least, a rollup should not contain two such PRs). Perhaps a strong waning in such cases would be useful?

I'm not sure this is a useful metric ... a PR that changes that many files is probably mass-renaming a common function, which is actually quite safe. Lines of code is a little better, but can be fooled by the same sort of change - lines of code within a file is more reliable, but can be tricked by moving code from one file to another. I think this is going to be really hard to automate in any useful way.

jyn514 commented 1 year ago

I don't think we should add any automation here until we agree what would actually be useful; or at least, agree that it's ok to turn it off if we're not finding it useful.