Skip to content
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

Jump to bottom

triagebot: automatically add more rustdoc related labels #133312

Merged
merged 1 commit into from
Feb 15, 2025
Merged

triagebot: automatically add more rustdoc related labels #133312

merged 1 commit into from
Feb 15, 2025

Conversation

lolbinarycat
Copy link
Contributor

@rustbot

This comment was marked as off-topic.

Sign in to view
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 21, 2024
@rustbot

This comment was marked as off-topic.

Sign in to view
@apiraino
Copy link
Contributor

apiraino commented Nov 22, 2024 •
edited
Loading

cc'ing @rust-lang/rustdoc

GuillaumeGomez
GuillaumeGomez reviewed Nov 22, 2024
View reviewed changes
triagebot.toml
@@ -230,6 +240,24 @@ trigger_files = [
"src/tools/jsondoclint",
]

[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

@GuillaumeGomez
Copy link
Member

Since it's something impacting the whole team, let's cc them too.

cc @rust-lang/rustdoc

@jieyouxu
Copy link
Member

r? @GuillaumeGomez (this affects rustdoc)

@jieyouxu jieyouxu added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Nov 22, 2024
@fmease
Copy link
Member

fmease commented Nov 22, 2024

No objections

@aDotInTheVoid
Copy link
Member

No objections from me, thanks!

@GuillaumeGomez
Copy link
Member

Ok. Just waiting for an answer to my question then I'll approve it. Thanks everyone!

@lolbinarycat
Copy link
Contributor Author

after reading through the source code to ensure it wouldn't produce undesired behavior, i added a trigger_files list for T-rustdoc-frontend.

@bors
Copy link
Collaborator

bors commented Feb 7, 2025

☔ The latest upstream changes (presumably #136684) made this pull request unmergeable. Please resolve the merge conflicts.

@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez has your question been answered? will you merge this after rebase?

@GuillaumeGomez
Copy link
Member

Yes, seems good to me.

@lolbinarycat lolbinarycat force-pushed the triagebot-rustdoc-labels branch from 7052ca5 to cc74ed0 Compare February 13, 2025 18:17
@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez rebased

@GuillaumeGomez
Copy link
Member

Thanks!

Let's give it a try then.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 14, 2025

📌 Commit cc74ed0 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 14, 2025
@jhpratt
Rollup merge of rust-lang#133312 - lolbinarycat:triagebot-rustdoc-lab…
4fa3318 
…els, r=GuillaumeGomez

triagebot: automatically add more rustdoc related labels

[inspired by zulip discussion](https://rust-lang.zulipchat.com/#narrow/channel/242269-t-release.2Ftriage/topic/auto-adding.20team.20labels.20based.20on.20certain.20other.20labels)
@jhpratt jhpratt mentioned this pull request Feb 14, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2025
@bors
Auto merge of rust-lang#137040 - jhpratt:rollup-lr17zya, r=jhpratt
a931943 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#133312 (triagebot: automatically add more rustdoc related labels)
 - rust-lang#134016 (Stabilize `const_is_char_boundary` and `const_str_split_at`.)
 - rust-lang#135813 (CI: split i686-mingw job to three free runners)
 - rust-lang#136879 (Add safe new() to NotAllOnes)
 - rust-lang#136971 (Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`)
 - rust-lang#136983 (Prepare standard library for Rust 2024 migration)
 - rust-lang#137002 (Fix early lint check desc in query)
 - rust-lang#137006 (borrowck diagnostics cleanup: remove an unused and a barely-used field)
 - rust-lang#137026 (Stabilize (and const-stabilize) `integer_sign_cast`)
 - rust-lang#137028 (mir_build: Clarify some code for lowering `hir::PatExpr` to THIR)
 - rust-lang#137032 (Decode metadata buffer in one go)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee workingjubilee mentioned this pull request Feb 14, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
@bors
Auto merge of rust-lang#137046 - workingjubilee:rollup-u56aw1m, r=wor…
54a0f38 
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#133312 (triagebot: automatically add more rustdoc related labels)
 - rust-lang#134016 (Stabilize `const_is_char_boundary` and `const_str_split_at`.)
 - rust-lang#136971 (Add a new check-pass UI test for returning `impl Fn(T) -> impl Trait`)
 - rust-lang#136983 (Prepare standard library for Rust 2024 migration)
 - rust-lang#137002 (Fix early lint check desc in query)
 - rust-lang#137006 (borrowck diagnostics cleanup: remove an unused and a barely-used field)
 - rust-lang#137032 (Decode metadata buffer in one go)
 - rust-lang#137035 (Normalize closure instance before eagerly monomorphizing it)
 - rust-lang#137037 (add x86-sse2 (32bit) ABI that requires SSE2 target feature)
 - rust-lang#137038 (llvm: Tolerate captures in tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 17ad8b5 into rust-lang:master Feb 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2025
@rust-timer
Unrolled build for rust-lang#133312
b8aeae2 
Rollup merge of rust-lang#133312 - lolbinarycat:triagebot-rustdoc-labels, r=GuillaumeGomez

triagebot: automatically add more rustdoc related labels

[inspired by zulip discussion](https://rust-lang.zulipchat.com/#narrow/channel/242269-t-release.2Ftriage/topic/auto-adding.20team.20labels.20based.20on.20certain.20other.20labels)
@lolbinarycat lolbinarycat mentioned this pull request Feb 15, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
9 participants
@lolbinarycat @rustbot @apiraino @GuillaumeGomez @jieyouxu @fmease @aDotInTheVoid @bors @Mark-Simulacrum