Skip to content

Allow glob matches for autolabels and rendered links - #2457

Merged
Urgau merged 5 commits into
rust-lang:masterfrom
tgross35:autolabel-glob
Jul 10, 2026
Merged

Allow glob matches for autolabels and rendered links#2457
Urgau merged 5 commits into
rust-lang:masterfrom
tgross35:autolabel-glob

Conversation

@tgross35

@tgross35 tgross35 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Use the same logic that currently exists for mentions to match autolabel.trigger-files and render-links.{trigger-files, exclude-files}.

The first commit just moves things around, the rest is less noisy.

Comment thread src/handlers/mentions.rs
@tgross35
tgross35 force-pushed the autolabel-glob branch 2 times, most recently from b950e2a to 80ddcb9 Compare July 9, 2026 17:25

@Urgau Urgau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. Left two nits.

View changes since this review

Comment thread src/utils.rs Outdated
Comment thread src/utils.rs
Comment on lines +116 to +117
// Create the glob pattern and log an error (should have already been reported to
// the user).

@Urgau Urgau Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this is adding glob patterns to other places, would you mind adding some validation for users in validate_config.rs, similar to this one:

// Error if one the mentions entry is not a valid glob.
if let Some(mentions) = config.mentions {
for (entry, MentionsEntryConfig { type_, .. }) in mentions.entries {
if type_ == MentionsEntryType::Filename {
if let Err(err) = globset::Glob::new(&entry) {
return Ok(Some(format!(
"Invalid `triagebot.toml`:\n\
`[mentions.\"{entry}\"]` has an invalid glob syntax: {err}"
)));
}
if entry.starts_with('/') {
return Ok(Some(format!(
"Invalid `triagebot.toml`:\n\
`[mentions.\"{entry}\"]` has an invalid pattern: path must be relative (remove the `/` at the start)"
)));
}
}
}
}

If you don't want to do it that's fine, just tell me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't know this existed, added a commit changing it. Added a commit changing this though I'm not quite sure how to test. (Feel free to just push if there's something I'm missing)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Testing requires a setting up triagebot on a repo and sending a PR changing traigebot.toml. I will test it for you.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works as expected!

Mentions:
image

Autolabel:
image

Rendered link:
image

Details
[autolabel."test"]
trigger_files = [
    "inva{id"
]

[rendered-link]
trigger-files = ["inva{id"]
exclude-files = ["inva{iid"]

[mentions."inval{id"]
cc = ["Urgau"]

The only sub-optimal thing is that we only show one error at a time, but that's pre-existing.

tgross35 added 2 commits July 9, 2026 22:43
Make it easier to match a set of files against a set of patterns without
needing to either collect or repeat the pattern building.
Use the same logic that currently exists for mentions to match
`autolabel.trigger-files` and `render-links.{trigger-files,
exclude-files}`.
Comment thread src/utils.rs Outdated

@Urgau Urgau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the change (and the improvements!).

View changes since this review

@Urgau
Urgau added this pull request to the merge queue Jul 10, 2026
Merged via the queue into rust-lang:master with commit e559c3e Jul 10, 2026
3 checks passed
@tgross35
tgross35 deleted the autolabel-glob branch July 10, 2026 18:06
@tgross35

Copy link
Copy Markdown
Contributor Author

Thanks!

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.

2 participants