Skip to content

Add T-compiler P-high issues triage module#2431

Open
apiraino wants to merge 1 commit into
rust-lang:masterfrom
apiraino:add-p-high-issues-triage-module
Open

Add T-compiler P-high issues triage module#2431
apiraino wants to merge 1 commit into
rust-lang:masterfrom
apiraino:add-p-high-issues-triage-module

Conversation

@apiraino

@apiraino apiraino commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

In an effort to make the triagebot more modular, I wanted to experiment with workspaces.

I need a new binary to run another meeting (the P-high issues review we do from time to time) and instead of adding code to the main triagebot codebase, I've tried creating a workspace ("utils", not a great name). Ideally I want to detach these binaries that now live under ./src/bin and make them as independent as possible. Their main dependency i think is the GitHub client.

Build with: cargo [ check | build ] --workspace

Unsure if this is the right direction, so happy to get feedback!

r? @Urgau (for a first opinion, thanks!)

Comment thread utils/compiler-p-high/src/main.rs Outdated
Comment on lines +6 to +9
use triagebot::actions::{Action, Query, QueryKind, QueryMap};
use triagebot::github::issue_query::Query as IssueQuery;
use triagebot::github::{GithubClient, Repository};
use triagebot::team_data::TeamClient;

@apiraino apiraino Jun 10, 2026

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.

So far, these are the dependencies on the triagebot. I'm thinking to move some code out of the triagebot that is used only for these binaries (triagebot::action::*) and pull the rest (triagebot::{github,team_data}) as a triagebot dependency.

team_data is a crate so easily detachable from the triagebot (right?). github ideally could become an external library.

Anyway, just thoughts for now.

View changes since the review

Comment thread utils/compiler-p-high/src/main.rs Outdated
use std::collections::HashMap;
use std::sync::{Arc, LazyLock};

use async_trait::async_trait;

@apiraino apiraino Jun 10, 2026

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.

Do we need this crate in 2026? I mean, don't we have something in stdlib?

View changes since the review

@apiraino apiraino Jun 10, 2026

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.

This is the only template I need here, I will cleanup the rest

View changes since the review

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.

I'm surprised we still have the T-lang templates. We don't have the generators anymore, we should remove them.

@Urgau

Urgau commented Jun 11, 2026

Copy link
Copy Markdown
Member

Unsure if this is the right direction, so happy to get feedback!

I think that as long as the main code, that is issue_query::Query and the actions module are in triagebot, having a workspace doesn't bring much compared to having individual binaries in bin/.

I think we should got a step further and completely rip-out all the bits and pieces required used the agendas (for the github client, I would switch to the octocrab crate).

That way the agenda generators are completely independent, and we wouldn't even need workspaces.

@apiraino

Copy link
Copy Markdown
Contributor Author

I think we should got a step further and completely rip-out all the bits and pieces required used the agendas (for the github client, I would switch to the octocrab crate).

Thanks! I also think this is the way to go (see triagebot#2403). I'll keep on working on this path (as a workspace) to get a feel of the umbilical cord with the triagebot, then I'll see if I can cut it completely.

@apiraino apiraino force-pushed the add-p-high-issues-triage-module branch 13 times, most recently from 7fe6ae4 to f0fadf1 Compare June 22, 2026 14:22
@rustbot

This comment has been minimized.

@apiraino apiraino force-pushed the add-p-high-issues-triage-module branch 10 times, most recently from b514dca to dca6240 Compare June 22, 2026 16:19
@apiraino apiraino force-pushed the add-p-high-issues-triage-module branch 3 times, most recently from fc4d97f to 36bc20f Compare June 23, 2026 17:18
@apiraino apiraino force-pushed the add-p-high-issues-triage-module branch from 36bc20f to 9e07145 Compare June 26, 2026 14:12
@apiraino apiraino marked this pull request as ready for review June 26, 2026 14:16
@apiraino

Copy link
Copy Markdown
Contributor Author

@Urgau when you have time, I think we can start reviewing this. You mentioned that you wanted some redundant code removed: do you still see anything I can pull out (I left come of the basic functions in the client intact).

Thanks

@apiraino apiraino requested a review from Urgau June 26, 2026 14:18
@Urgau

Urgau commented Jul 5, 2026

Copy link
Copy Markdown
Member

Th extracted parts looks fine, though I'm wondering what the next steps would be? Only using octocrab (ie. removing our own GitHub client)? Porting the other agendas?

I'm also wondering if we even need a workspace member since the new logic doesn't link directly to triagebot, we could also better name the folder, maybe agendas/?

do you still see anything I can pull out

I think Repository::submodule, PullRequestDetails::compare (and all the related GitHubCommit, FileDiff structs and such).

@apiraino

apiraino commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

I think we cannot just yet use octocrab, there are 330 SLOC in ./utils/compiler-p-high/src/github/client.rs that I think cannot be replaced by the crate. Ideally an outcome of this experimentation could be trying to upstream some of our stuff into octocrab (if it's not too custom and ofc if the maintainer agrees 😉 ).

The GH client for the compiler triage meeting (prioritization_agenda.rs) is likely be much bigger, there is all the GraphQL code.

The workspace was meant to dramatically shave off compile time, which is one of my annoyances when touching these agenda generators: but I still don't have numbers to support this claim.

(agree on the file naming, thanks for the suggestion)

Thanks for your comments, it was really helpful. I'll circle back to this hopefully in the near future. Now I have to park this for a bit.

@Urgau

Urgau commented Jul 7, 2026

Copy link
Copy Markdown
Member

Given that octocrab allow custom HTTP requests, I think we could entirely remove our own custom GitHub client (for agendas and the current triagebot).

(The only thing we would be losing if we only used octocrab client is the automatic retry of requests, but we don't use that currently so.)

Same for the GraphQl queries, we could use the graphql method with our custom queries.

Anyway, those are just some things I noticed while quickly looking at the documentation.

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.

3 participants