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

chazkiker2 · 2022-03-23T00:31:26Z

Description

Fixes lang agenda: include concerns and FCP notes #1592

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

chazkiker2 · 2022-03-23T00:31:51Z

@rustbot claim

src/github.rs

chazkiker2 · 2022-03-23T02:33:33Z

r? @nikomatsakis @Mark-Simulacrum

Concerned about a couple things:

See this comment: https://github.com/rust-lang/triagebot/pull/1596/files#r832791113
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
Would love your guidance @Mark-Simulacrum on the "where everything lives" aspect of this... feels like the engineering could be improved quite a bit

src/github.rs

chazkiker2 · 2022-03-23T03:06:26Z

src/github.rs

+                        let initiating_comment_html_url =
+                            format!("{}#issuecomment-{}", issue.html_url, fk_initiating_comment,);
+                        let init_comment = issue
+                            .get_comment(&client, fk_initiating_comment.try_into()?)


should I just make IssueDecorators.fpc_details.fk_initiating_comment a usize from the get-go?

src/github.rs

chazkiker2 · 2022-03-23T03:09:06Z

templates/_issues_fcps.tt

+- [**Tracking Comment**]({{issue.fcp_details.bot_tracking_comment_html_url}}): {{issue.fcp_details.bot_tracking_comment_content}}
+- [**Initiating Comment**]({{issue.fcp_details.initiating_comment_html_url}}): {{issue.fcp_details.initiating_comment_content}}


wondering what happens if issue.fcp_details is passed in as None

nikomatsakis · 2022-03-23T13:35:42Z

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.

nikomatsakis

Overall, seems kind of like I expected! Left some nits and questions.

src/agenda.rs

src/github.rs

nikomatsakis · 2022-03-23T13:39:25Z

src/github.rs

+                        );
+                        let bot_tracking_comment_content = quote_reply(&fcp.status_comment.body);
+                        let fk_initiating_comment = fcp.fcp.fk_initiating_comment;
+                        let initiating_comment_html_url =


probably orthogonal, but I'm surprised we don't use some library for this sort of thing

are you referring to accessing the comment link or to the quote_reply function?

unrelated but I'm similarly surprised we're using a home-baked parser rather than a command line library (like clap) to take in commands — surely wouldn't be too hard to treat a github comment like command line input

I meant something like https://crates.io/crates/octocrab

It is kind of interesting, especially b/c octocrab is in here, but nothing in src/github.rs seems to use it

triagebot/src/handlers.rs

Lines 250 to 255 in 2a37c45

pub struct Context {

pub github: GithubClient,

pub db: crate::db::ClientPool,

pub username: String,

pub octocrab: Octocrab,

}

src/github.rs

…plate

Mark-Simulacrum · 2022-04-12T23:07:53Z

@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 · 2022-04-12T23:56:45Z

@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 · 2022-04-13T00:03:33Z

Yeah, it seems a little messy, but since we don't have obvious next steps we can probably merge. With #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 · 2022-04-13T00:06:02Z

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

chazkiker2 · 2022-04-13T01:04:58Z

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

chazkiker2 added 4 commits March 21, 2022 21:20

very broken, just playing around

bf26ff7

generate all relevant information for FCPDecorator

73f11c5

introduce template changes for _issues_fcps.tt

0e84825

clean up lints & print statements

b8f4990

rustbot assigned chazkiker2 Mar 23, 2022

chazkiker2 added 3 commits March 22, 2022 18:44

further simplify code; move FCPDecorator to actioins

d5af742

get initializing comment content

851ac34

clean up code

fb496d2

chazkiker2 commented Mar 23, 2022

View reviewed changes

src/github.rs Outdated Show resolved Hide resolved

chazkiker2 marked this pull request as ready for review March 23, 2022 02:33

chazkiker2 commented Mar 23, 2022

View reviewed changes

nikomatsakis reviewed Mar 23, 2022

View reviewed changes

chazkiker2 added 4 commits March 23, 2022 18:57

revert formatting changes in src/agenda.rs

cdc9231

change 'into_iter' => 'iter'

2388ab5

use existing comment.html_url; remove unnecessary whitespace from tem…

986d542

…plate

remove 'async move' block; instead, just iterate through issues directly

c7d2261

Mark-Simulacrum merged commit 0fc41f5 into rust-lang:master Apr 13, 2022

chazkiker2 deleted the feature/proposed-fcp-details branch April 13, 2022 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

chazkiker2 commented Mar 23, 2022 •

edited

Loading

chazkiker2 commented Mar 23, 2022

chazkiker2 commented Mar 23, 2022

chazkiker2 Mar 23, 2022

chazkiker2 Mar 23, 2022

nikomatsakis commented Mar 23, 2022

nikomatsakis left a comment

nikomatsakis Mar 23, 2022

chazkiker2 Mar 24, 2022

chazkiker2 Mar 24, 2022

nikomatsakis Mar 24, 2022

chazkiker2 Mar 24, 2022

Mark-Simulacrum commented Apr 12, 2022

chazkiker2 commented Apr 12, 2022

Mark-Simulacrum commented Apr 13, 2022

chazkiker2 commented Apr 13, 2022

chazkiker2 commented Apr 13, 2022

		- [Tracking Comment]({{issue.fcp_details.bot_tracking_comment_html_url}}): {{issue.fcp_details.bot_tracking_comment_content}}
		- [Initiating Comment]({{issue.fcp_details.initiating_comment_html_url}}): {{issue.fcp_details.initiating_comment_content}}

	pub struct Context {
	pub github: GithubClient,
	pub db: crate::db::ClientPool,
	pub username: String,
	pub octocrab: Octocrab,
	}

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

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

Conversation

chazkiker2 commented Mar 23, 2022 • edited Loading

Description

chazkiker2 commented Mar 23, 2022

chazkiker2 commented Mar 23, 2022

chazkiker2 Mar 23, 2022

Choose a reason for hiding this comment

chazkiker2 Mar 23, 2022

Choose a reason for hiding this comment

nikomatsakis commented Mar 23, 2022

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Mar 23, 2022

Choose a reason for hiding this comment

chazkiker2 Mar 24, 2022

Choose a reason for hiding this comment

chazkiker2 Mar 24, 2022

Choose a reason for hiding this comment

nikomatsakis Mar 24, 2022

Choose a reason for hiding this comment

chazkiker2 Mar 24, 2022

Choose a reason for hiding this comment

Mark-Simulacrum commented Apr 12, 2022

chazkiker2 commented Apr 12, 2022

Mark-Simulacrum commented Apr 13, 2022

chazkiker2 commented Apr 13, 2022

chazkiker2 commented Apr 13, 2022

chazkiker2 commented Mar 23, 2022 •

edited

Loading