search

rust-lang / triagebot

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

[feature] Lang Agenda — Proposed FCPs: include concerns and FCP notes #1596

Closed chazkiker2 closed 2 years ago

chazkiker2 commented 2 years ago

Description

See an example of the new lang agenda (with updated FCP details) in this HackMD

chazkiker2 commented 2 years ago

@rustbot claim

chazkiker2 commented 2 years ago

r? @nikomatsakis @Mark-Simulacrum

Concerned about a couple things:

  1. See this comment: https://github.com/rust-lang/triagebot/pull/1596/files#r832791113
  2. Seems like I trip GitHub's rate limit within just a handful of runs — may have just been b/c I was running locally over and over but it could also be a problem
  3. Would love your guidance @Mark-Simulacrum on the "where everything lives" aspect of this... feels like the engineering could be improved quite a bit
nikomatsakis commented 2 years ago

Seems like I trip GitHub's rate limit within just a handful of runs — may have just been b/c I was running locally over and over but it could also be a problem

That's not great. That said, I typically run this script once per week...

...still, I wonder if there's a way to combine this into a single graphql query or something.

Mark-Simulacrum commented 2 years ago

@chazkiker2 Can you say if this seems ready to go on your end? My feeling is that we should probably go ahead and merge presuming it mostly works, and then if there's bugs to be fixed or rate limiting etc we can either revert or fix those depending on complexity.

chazkiker2 commented 2 years ago

@Mark-Simulacrum shoot — sorry, kinda lost track of this.

If you think the include_fcp_details param is fine where it is for now as per this comment, then I think this is pretty much ready to move forward even though I feel like it's not quite as refined as I wish it were...

Mark-Simulacrum commented 2 years ago

Yeah, it seems a little messy, but since we don't have obvious next steps we can probably merge. With https://github.com/rust-lang/triagebot/issues/1599 there might be simplification possible but seems also a little difficult to put a finger on.

I'll merge this tomorrow to let some of the changes I merged today go live first, I think.

chazkiker2 commented 2 years ago

@Mark-Simulacrum okay sounds like a decent plan :). sorry for the slowness!

chazkiker2 commented 2 years ago

@Mark-Simulacrum btw i don’t think i have merge access on this repo, so feel free to merge in whenever you’re ready