Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,9 @@ pub(crate) struct MajorChangeConfig {
pub(crate) zulip_stream: u64,
/// Extra text in the opening major change.
pub(crate) open_extra_text: Option<String>,
/// Template for a tracking issue to be created when the major change is accepted
#[serde(rename = "tracking-issue-template")]
pub(crate) tracking_issue_template: Option<MajorChangeTrackingIssueTemplateConfig>,
}

impl MajorChangeConfig {
Expand All @@ -433,6 +436,20 @@ impl MajorChangeConfig {
}
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(rename_all = "kebab-case")]
#[serde(deny_unknown_fields)]
pub(crate) struct MajorChangeTrackingIssueTemplateConfig {
/// Template for the title
pub(crate) title: String,
/// Repository where to create the tracking issue (otherwise the current repository)
pub(crate) repository: Option<String>,
/// List of labels to add to the issue
pub(crate) labels: Vec<String>,
/// Template of the body
pub(crate) body: String,
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct CloseConfig {}
Expand Down Expand Up @@ -1030,4 +1047,50 @@ mod tests {
})
));
}

#[test]
fn major_change() {
let config = r#"
[major-change]
enabling_label = "major-change"
meeting_label = "to-announce"
second_label = "final-comment-period"
concerns_label = "has-concerns"
accept_label = "major-change-accepted"
waiting_period = 1
auto_closing = true
zulip_stream = 224082
zulip_ping = "Urgau"

[major-change.tracking-issue-template]
repository = "triagebot"
title = "Tracking issue for MCP#${mcp_number}"
body = """
Multi text body with ${mcp_issue} and ${mcp_title}
"""
labels = ["C-tracking-issue", "T-compiler"]
"#;
let config = toml::from_str::<Config>(&config).unwrap();
assert_eq!(
config.major_change,
Some(MajorChangeConfig {
zulip_ping: "Urgau".to_string(),
enabling_label: "major-change".to_string(),
second_label: "final-comment-period".to_string(),
accept_label: "major-change-accepted".to_string(),
meeting_label: "to-announce".to_string(),
concerns_label: Some("has-concerns".to_string()),
waiting_period: 1,
auto_closing: true,
zulip_stream: 224082,
open_extra_text: None,
tracking_issue_template: Some(MajorChangeTrackingIssueTemplateConfig {
title: "Tracking issue for MCP#${mcp_number}".to_string(),
repository: Some("triagebot".to_string()),
body: "Multi text body with ${mcp_issue} and ${mcp_title}\n".to_string(),
labels: vec!["C-tracking-issue".to_string(), "T-compiler".to_string()],
})
})
);
}
}
62 changes: 54 additions & 8 deletions src/handlers/major_change.rs
Copy link
Member

@fmease fmease Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks!

I don't want to smother you with extra work, so these things can be done as follow-ups (or not all :P): In my issue & MCP I also proposed to

  • have some suppress-auto-tracking label to suppress the creation of the tracking issue.

    So it would be pretty great if one was able to register such a label in [major-change.tracking-issue-template] assuming you agree that such a mechanism would be useful (IIRC @Kobzol doubted the usefulness).

    That could come in clutch in situations like open MCP Remove diagnostic translation infrastructure compiler-team#924 where we already have a tracking issue: Tracking Issue for rustc's translatable diagnostics infrastructure rust#132181 (sort of). And IIRC my MCP / issue had some other vague examples?

  • have a way to ping the MCP author (and registered mentors if technically feasible) since the link to the MCP can be easily missed; however, it's probably not worth the effort

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • have some suppress-auto-tracking label to suppress the creation of the tracking issue

While I don't doubt the usefulness in some scenario I think they are going to be extremely limited. Even in your example I would still want to have the tracking issue created as they wouldn't track the same thing.

If we see the need for it we can always add it later.

  • have a way to ping the MCP author

Done, added the ${mcp_author} variable to get the handle of the MCP issue author.

  • and registered mentors if technically feasible

That would require some sort of easily-parseble format for triagebot, it's currently a free for all. Let's defer that for now.

Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ impl Job for MajorChangeAcceptenceJob {

let now = Utc::now();

match process_seconded(ctx, &major_change, now).await {
match try_accept_mcp(ctx, &major_change, now).await {
Ok(()) => {
tracing::info!(
"{}: major change ({:?}) as been accepted",
Expand Down Expand Up @@ -549,7 +549,7 @@ impl Job for MajorChangeAcceptenceJob {
}
}

async fn process_seconded(
async fn try_accept_mcp(
ctx: &super::Context,
major_change: &MajorChangeSeconded,
now: DateTime<Utc>,
Expand Down Expand Up @@ -581,17 +581,18 @@ async fn process_seconded(
.await
.context("unable to get the associated issue")?;

let (org, repo) = major_change
.repo
.split_once('/')
.context("unable to split org/repo")?;

{
// Static checks against the timeline to block the acceptance if the state changed between
// the second and now (like concerns added, issue closed, ...).
//
// Note that checking the timeline is important as a concern could have been added and
// resolved by the time we run, missing that this job should not run.

let (org, repo) = major_change
.repo
.split_once('/')
.context("unable to split org/repo")?;
let timeline = ctx
.octocrab
.issues(org, repo)
Expand Down Expand Up @@ -665,13 +666,58 @@ async fn process_seconded(
}

if !issue.labels.iter().any(|l| l.name == config.accept_label) {
// Only post the comment if the accept_label isn't set yet, we may be in a retry
// Only create the tracking issue and post the comment if the accept_label isn't set yet, we may be in a retry.

let tracking_issue_text =
if let Some(tracking_issue_template) = &config.tracking_issue_template {
let issue_repo = crate::github::IssueRepository {
organization: org.to_string(),
repository: tracking_issue_template
.repository
.as_deref()
.unwrap_or(repo)
.to_string(),
};

let fill_out = |input: &str| {
input
.replace("${mcp_number}", &issue.number.to_string())
.replace("${mcp_title}", &issue.title)
.replace("${mcp_author}", &issue.user.login)
};

let title = fill_out(&tracking_issue_template.title);
let body = fill_out(&tracking_issue_template.body);

let tracking_issue = ctx
.github
.new_issue(
&issue_repo,
&title,
&body,
tracking_issue_template.labels.clone(),
)
.await
.context("unable to create MCP tracking issue")
.with_context(|| format!("title: {title}"))
.with_context(|| format!("body: {body}"))?;

format!(
r"
Progress of this major change will be tracked in the tracking issue at {}#{}.
",
&issue_repo, tracking_issue.number
)
} else {
String::new()
};

issue
.post_comment(
&ctx.github,
&format!(
r"The final comment period is now complete, this major change is now **accepted**.

{tracking_issue_text}
As the automated representative, I would like to thank the author for their work and everyone else who contributed to this major change proposal.

*If you think this major change shouldn't have been accepted, feel free to remove the `{}` label and reopen this issue.*",
Expand Down