Skip to content

triagebot: automatically add more rustdoc related labels #133312

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 15, 2025
Merged

triagebot: automatically add more rustdoc related labels #133312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 37 additions & 0 deletions triagebot.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,16 @@ exclude_labels = [
"T-*",
]

trigger_labels = [
"A-rustdoc-json",
"A-rustdoc-type-layout",
"A-rustdoc-scrape-examples",
"A-link-to-definition",
"A-cross-crate-reexports",
"A-intra-doc-links",
"A-doc-alias",
]

[autolabel."A-rustdoc-json"]
trigger_files = [
"src/librustdoc/json/",
Expand All @@ -245,6 +255,33 @@ trigger_files = [
"compiler/rustc_attr_validation",
]

[autolabel."T-rustdoc-frontend"]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't there be trigger_files here too?

Copy link
Member

@jieyouxu jieyouxu Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is fine because it will transitively trigger due to the A-* labels themselves getting triggered on files. I.e. this T-rustdoc-frontend label triggers on A-rustdoc-search but A-rustdoc-search triggers on a set of paths.

... or so I hope triagebot does (transitive trigger files)...

Copy link
Member

Choose a reason for hiding this comment

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

I think it's not great to rely on transitive triggers, especially when we remove a "parent". Then children won't trigger it anymore and we'll only discover it anymore. Would be nice if we could say that a label depends on another directly.

Copy link
Member

@jieyouxu jieyouxu Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, probably safer to duplicate the relevant path filters here. Also btw, you can just edit this PR locally to add the path filters you want :)

Copy link
Member

Choose a reason for hiding this comment

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

I know but since I'm not the owner here, I'll wait for them to update. If they don't want to, I can always send a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My biggest concern is that T-rustdoc and T-rustdoc-frontend are mutually exclusive, and the paths that would trigger the frontend label are in dirs that trigger the T-rustdoc label.

i'm not sure if the longer path would take precedent, or if it would depend on hashmap ordering, or what

Copy link
Member

Choose a reason for hiding this comment

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

I thought they would all be triggered...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, that's an option i hadn't considered, adding both T-rustdoc and T-rustdoc-frontend, even though they're supposed to be mutually exclusive...

hmm... i suppose it's unlikely to be anything destructive, so if it does cause an issue, it should be pretty easy to fix...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after reading through the relevant code, it appears that all autolabel rules apply simultaneously, so it would indeed add both. I'll add the trigger_files

trigger_labels = [
"A-rustdoc-search",
"A-rustdoc-ui",
"A-rustdoc-js",
]

trigger_files = [
"src/librustdoc/html/",
"tests/rustdoc/",
"tests/rustdoc-gui/",
"tests/rustdoc-js/",
"tests/rustdoc-js-std/",
# note: tests/rustdoc-ui tests the CLI, not the web frontend
]

[autolabel."A-rustdoc-search"]
trigger_files = [
"src/librustdoc/html/static/js/search.js",
"tests/rustdoc-js",
"tests/rustdoc-js-std",
]

trigger_labels = [
"A-type-based-search",
]

[autolabel."T-compiler"]
trigger_files = [
# Source code
Expand Down
Loading