Skip to content

Auto-nominate for backport a pull request fixing a regression #2092

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

apiraino
Copy link
Contributor

@apiraino apiraino commented Jun 30, 2025

From #1184:

We should nominate PRs for backport if any of the following are true:

  1. Description mentions a issue which is labeled as [regression]
  2. Description mentions a issue which is milestoned with the beta version number

This patch implements point (1) and adds a check when a new PR is opened. The new backport.rs handler tries to figure to out if a PR should be nominated for backport.

This handler is enabled by adding a new [backport] config item. The format allows multiple instances of the config so teams can handle backports as they wish.

# "foo" is just to disambiguate multiple instances of this config
[backport.foo]
# The pull request MUST have one of these labels
required_pr_labels = ["T-compiler"]
# The regression MUST have this label
required_issue_labels = "regression-from-stable-to-beta"
# if the above conditions matches, the PR will receive these labels
add_labels = ["beta-nominated"]

[backport.bar]
required_pr_labels = ["T-compiler"]
required_issue_labels = "regression-from-stable-to-stable"
add_labels = ["beta-nominated", "stable-nominated"]

[backport.baz]
required_pr_labels = ["T-libs", "T-libs-api"]
required_issue_labels = "regression-from-stable-to-stable"
add_labels = ["stable-nominated"]

The workflow that models the decision to add the backport label is:

  1. Someone files a regression #123, with a t-label (ex. "T-compiler")
  2. Contributor opens PR #456, adds comment "Fixes #123"
  3. triagebot receives the event "new PR"
  4. triagebot parses the opening comment and retrieves originating regression(s)
    3a. checks if PR has the required team labels (reads from required_pr_labels), loads the appropriate config
    3b. checks if issue has a regression-from-* label (reads from required_issue_labels)
    3c. checks the P-label of the regression, discard P-low and P-medium regressions
  5. triagebot adds backport label(s) to PR Implement the prioritize command #456 (reads from add_labels)

TODOs

Situations not yet handled where this handler should trigger as well:

  • PR is opened without a T-label (can that happen? or is that automatically applied?), user adds a T-label to the PR later
  • The regression receives a P-label when the PR fixing it already exists
  • The expected marker text ("Fixes, resolves, etc.") is added later
  • probably other cases ...

This work supersedes #1380 (Important: before merge this commit should be Co-authored1)

r?

Footnotes

  1. Not doing now to avoid spamming the original author with multiple forced git push 🙂

@rust-lang rust-lang deleted a comment from rustbot Jun 30, 2025
@apiraino apiraino force-pushed the auto-nominate-backports branch 7 times, most recently from 7a37ed9 to 9167286 Compare July 8, 2025 14:23
@apiraino apiraino force-pushed the auto-nominate-backports branch 2 times, most recently from 2484587 to eb5b382 Compare July 9, 2025 14:45
@apiraino apiraino requested a review from Urgau July 9, 2025 14:48
@apiraino apiraino force-pushed the auto-nominate-backports branch 2 times, most recently from dc9450f to 4f7b659 Compare July 14, 2025 12:23
@apiraino apiraino force-pushed the auto-nominate-backports branch from 4f7b659 to 9682868 Compare July 16, 2025 11:46
@apiraino apiraino force-pushed the auto-nominate-backports branch 3 times, most recently from 18cd1ce to 71d50d8 Compare July 21, 2025 11:37
@Urgau
Copy link
Member

Urgau commented Jul 21, 2025

Modulo the last nit, this looks good to me. Thanks for putting it through my reviews.

Next step (preferably before merging) will be to open a PR against the forge to add documentation for this feature, with a description of what it does, it's currently limitations, how to activate it, ... (take inspiration from the current pages).

Regarding deployment, do you want to go straight to "prod" in rust-lang/rust or do you want to use this repo as a small playground before using it rust-lang/rust?

@apiraino
Copy link
Contributor Author

Next step (preferably before merging) will be to open a PR against the forge to add documentation for this feature, with a description of what it does, it's currently limitations, how to activate it, ... (take inspiration from the current pages).

Started this some time ago. I will look at it again and un-draft it

Regarding deployment, do you want to go straight to "prod" in rust-lang/rust or do you want to use this repo as a small playground before using it rust-lang/rust?

I did some tests in my "playground". Testing this PR here implies adding a whole bunch of labels that we don't use here (T-, regression-, *-backport). Unless I undo all the setup, it would be just dirt in the repository. I don't know. If anyone feels strongly about it, I will do it :)

@apiraino apiraino force-pushed the auto-nominate-backports branch from 71d50d8 to 1ec7a03 Compare July 22, 2025 14:34
@Urgau
Copy link
Member

Urgau commented Jul 22, 2025

I did some tests in my "playground".

Great. I'm find going to rust-lang/rust then.

Will merge the PR after you format the code (to fix CI).

@apiraino
Copy link
Contributor Author

apiraino commented Jul 23, 2025

Will merge the PR after you format the code (to fix CI).

Oh, I didn't notice the CI rustfmt failure. In this repo we have a config file that tells rustfmt to use edition 2018 but I think the CI is using the 2024 edition formatting options:

$ rustfmt --check --edition 2018 backport.rs

# same error reported by CI 
$ rustfmt --check --edition 2024 backport.rs
Diff in backport.rs:115:
         if let Some(org_repo) = caps.name("org_repo")
             && org_repo.as_str() != event.repository.full_name
         {
-            log::info!("Skipping backport nomination: Ignoring issue#{id} pointing to a different git repository: Expected {0}, found {org_repo:?}", event.repository.full_name);
+            log::info!(
+                "Skipping backport nomination: Ignoring issue#{id} pointing to a different git repository: Expected {0}, found {org_repo:?}",
+                event.repository.full_name
+            );
             continue;
         }
         input.ids.push(id);

Hmm is there a discrepancy between the rustfmt.toml and the CI? 🤔

@Urgau
Copy link
Member

Urgau commented Jul 23, 2025

(I'm not at a computer to verify this, so I may be wrong). I think cargo is overridding the edition in rustfmt.toml by the one in the Cargo.toml. I always just do cargo fmt locally, so I never saw the problem.

I think the rustfmt.toml is oudated and no longer used, we should remove it.

Co-authored-by: mibac138 <5672750+mibac138@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

3 participants