triagebot: automatically add more rustdoc related labels #133312
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
rustbot
added
A-meta
This comment was marked as off-topic.
This comment was marked as off-topic.
|
cc'ing @rust-lang/rustdoc |
| @@ -230,6 +240,24 @@ trigger_files = [ | |||
| "src/tools/jsondoclint", | |||
| ] | |||
|
|
|||
| [autolabel."T-rustdoc-frontend"] | |||
There was a problem hiding this comment.
Shouldn't there be trigger_files here too?
There was a problem hiding this comment.
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)...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I thought they would all be triggered...
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
|
Since it's something impacting the whole team, let's cc them too. cc @rust-lang/rustdoc |
|
r? @GuillaumeGomez (this affects rustdoc) |
jieyouxu
added
T-rustdoc
|
No objections |
|
No objections from me, thanks! |
|
Ok. Just waiting for an answer to my question then I'll approve it. Thanks everyone! |
|
after reading through the source code to ensure it wouldn't produce undesired behavior, i added a |
inspired by zulip discussion