rust-lang / triagebot

Automation/tooling for Rust spaces
https://triage.rust-lang.org
Apache License 2.0
170 stars 74 forks source link

[WG-prio agenda] Improve filter for T-compiler PRs s-waiting-for-review #1654

Closed apiraino closed 1 year ago

apiraino commented 1 year ago

By adding the "T-compiler" filter straight into the GraphQL query (instead of filtering them post-query) it seems we are getting a better "ranking" for this label - some really old PRs are emerging.

Side effect: by building the GraphQL query with 2 label filters instead of one, I get a 502 error when asking for 100 items per page. Reducing to 10 items per page and running 10 queries seems to workaround the issue.

r? @Mark-Simulacrum cc: @jackh726 (introduced GraphQL queries so may have additional comments)

thanks for a review

jackh726 commented 1 year ago

I think you're getting older PRs that have been reviewed more recently than newer PRs now, because of the change commented above. I don't think this is because of the added T-compiler label.

apiraino commented 1 year ago

I think you're getting older PRs that have been reviewed more recently than newer PRs now

it's true, the filter as it is in this patch is wrong. Still, I was surprised that I get now some very old PRs. I need to circle back on this and see if I can adjust the filter in the gql query + the post-query filtering (i.e. remove the uninteresting results) to improve the final list.

What I'm trying to improve (when compiling this item of the T-compiler agenda) is that I always see at the top the same PRs and some of them are not relevant to the meeting.

jackh726 commented 1 year ago

So, the question is why they're always on top? Are they getting "reviewed"? If so, then seems like a bug. If not, then seems correct.

apiraino commented 1 year ago

I'm gonna close this because it's going nowhere. Even if I tweak the GraphQL query I get more or less the same results from github. Some PRs stay on top of the list because they're not updated by neither the reviewer or the author, others are waiting on other teams.

It's hard to handle these special cases without fiddling with the PRs labels (which would add background noise, so I'd like to avoid that). For the time being, I'm afraid this bit of the T-compiler meeting agenda cannot be greatly improved/automated and will need a manual curation :-)

jackh726 commented 1 year ago

Is it not worth landing just adding the T-compiler label to the graphql query for rate-limiting reduction?

apiraino commented 1 year ago

hm good point @jackh726 I will pick some bits from this branch, test if I see measurable advantages, if yes will submit a more scoped patch. thanks :)