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.

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

Use rfcbot style for FCPs in the prioritization agenda #1784

Merged
merged 8 commits into from
Apr 23, 2024
20 changes: 19 additions & 1 deletion src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,29 @@ pub struct IssueDecorator {
pub mcp_details: Option<MCPDetails>,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct FCPConcernDetails {
pub name: String,
pub reviewer_login: String,
pub concern_url: String,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct FCPReviewerDetails {
pub github_login: String,
pub zulip_id: Option<u64>,
}

#[derive(Serialize, Deserialize, Debug)]
pub struct FCPDetails {
pub bot_tracking_comment_html_url: String,
pub bot_tracking_comment_content: String,
pub initiating_comment_html_url: String,
pub initiating_comment_content: String,
pub disposition: String,
pub should_mention: bool,
pub pending_reviewers: Vec<FCPReviewerDetails>,
pub concerns: Vec<FCPConcernDetails>,
}

#[derive(Serialize, Deserialize, Debug)]
Expand Down Expand Up @@ -130,6 +147,7 @@ impl<'a> Action for Step<'a> {
let query = query.clone();
handles.push(tokio::task::spawn(async move {
let _permit = semaphore.acquire().await?;
let fcps_groups = ["proposed_fcp", "in_pre_fcp", "in_fcp"];
let mcps_groups = [
"mcp_new_not_seconded",
"mcp_old_not_seconded",
Expand All @@ -140,7 +158,7 @@ impl<'a> Action for Step<'a> {
let issues = query
.query(
&repository,
name == "proposed_fcp",
fcps_groups.contains(&name.as_str()),
mcps_groups.contains(&name.as_str())
&& repository.full_name.contains("rust-lang/compiler-team"),
&gh,
Expand Down
52 changes: 51 additions & 1 deletion src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,8 @@ pub struct Comment {
pub html_url: String,
pub user: User,
#[serde(alias = "submitted_at")] // for pull request reviews
pub created_at: chrono::DateTime<Utc>,
#[serde(alias = "submitted_at")] // for pull request reviews
pub updated_at: chrono::DateTime<Utc>,
#[serde(default, rename = "state")]
pub pr_review_state: Option<PullRequestReviewState>,
Expand Down Expand Up @@ -1712,11 +1714,19 @@ impl<'q> IssuesQuery for Query<'q> {
.with_context(|| "Unable to get issues.")?;

let fcp_map = if include_fcp_details {
crate::rfcbot::get_all_fcps().await?
crate::rfcbot::get_all_fcps()
.await
.with_context(|| "Unable to get all fcps from rfcbot.")?
} else {
HashMap::new()
};

let zulip_map = if include_fcp_details {
Some(crate::team_data::zulip_map(client).await?)
} else {
None
};

let mut issues_decorator = Vec::new();
let re = regex::Regex::new("https://github.com/rust-lang/|/").unwrap();
let re_zulip_link = regex::Regex::new(r"\[stream\]:\s").unwrap();
Expand Down Expand Up @@ -1745,11 +1755,51 @@ impl<'q> IssuesQuery for Query<'q> {
.get_comment(&client, fk_initiating_comment.try_into()?)
.await?;

// TODO: agree with the team(s) a policy to emit actual mentions to remind FCP
// voting member to cast their vote
let should_mention = false;
Some(crate::actions::FCPDetails {
bot_tracking_comment_html_url,
bot_tracking_comment_content,
initiating_comment_html_url: init_comment.html_url.clone(),
initiating_comment_content: quote_reply(&init_comment.body),
disposition: fcp
.fcp
.disposition
.as_deref()
.unwrap_or("<unknown>")
.to_string(),
should_mention,
pending_reviewers: fcp
Urgau marked this conversation as resolved.
Show resolved Hide resolved
.reviews
.iter()
.filter_map(|r| {
(!r.approved).then(|| crate::actions::FCPReviewerDetails {
github_login: r.reviewer.login.clone(),
zulip_id: zulip_map
.as_ref()
.map(|map| {
map.users
.iter()
.find(|&(_, &github)| github == r.reviewer.id)
.map(|v| *v.0)
})
.flatten(),
})
})
.collect(),
concerns: fcp
.concerns
.iter()
.map(|c| crate::actions::FCPConcernDetails {
name: c.name.clone(),
reviewer_login: c.reviewer.login.clone(),
concern_url: format!(
"{}#issuecomment-{}",
issue.html_url, c.comment.id
),
})
.collect(),
})
} else {
None
Expand Down
11 changes: 9 additions & 2 deletions src/rfcbot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct FCP {
}
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct Reviewer {
pub id: u32,
pub id: u64,
pub login: String,
}
#[derive(Serialize, Deserialize, Debug, Clone)]
Expand All @@ -24,10 +24,16 @@ pub struct Review {
pub approved: bool,
}
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct Concern {
pub name: String,
pub comment: StatusComment,
pub reviewer: Reviewer,
}
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct FCPIssue {
pub id: u32,
pub number: u32,
pub fk_milestone: Option<String>,
pub fk_milestone: Option<u32>,
pub fk_user: u32,
pub fk_assignee: Option<u32>,
pub open: bool,
Expand Down Expand Up @@ -57,6 +63,7 @@ pub struct StatusComment {
pub struct FullFCP {
pub fcp: FCP,
pub reviews: Vec<Review>,
pub concerns: Vec<Concern>,
pub issue: FCPIssue,
pub status_comment: StatusComment,
}
Expand Down
15 changes: 15 additions & 0 deletions templates/_issues_rfcbot.tt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{% import "_issue.tt" as issue %}

{% macro render(issues, indent="", empty="None.") %}
{%- for issue in issues %}
{%- if issue.fcp_details is object %}
{{indent}}- {{issue.fcp_details.disposition}}: [{{issue.title}} ({{issue.repo_name}}#{{issue.number}})]({{issue.fcp_details.bot_tracking_comment_html_url}})
{{indent}}{{indent}}-{% for reviewer in issue.fcp_details.pending_reviewers %} @{% if issue.fcp_details.should_mention %}{% else %}_{% endif %}**|{{reviewer.zulip_id}}**{%else%} no pending checkboxs{% endfor %}
{{indent}}{{indent}}-{% if issue.fcp_details.concerns|length > 0 %} concerns:{% endif %}{% for concern in issue.fcp_details.concerns %} [{{concern.name}} (by {{concern.reviewer_login}})]({{concern.concern_url}}){%else%} no pending concerns{% endfor -%}
{% else %}
{{indent}}- {{issue::render(issue=issue)}}
{%- endif -%}
{%else%}
{{empty}}
{%endfor-%}
{% endmacro %}
5 changes: 3 additions & 2 deletions templates/prioritization_agenda.tt
Urgau marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{% import "_issues.tt" as issues %}
{% import "_meetings.tt" as meetings %}
{% import "_issues_rfcbot.tt" as issues_rfcbot %}

---
tags: weekly, rustc
Expand Down Expand Up @@ -28,9 +29,9 @@ note_id: xxx
- Old MCPs (not seconded, take a look)
{{-issues::render(issues=mcp_old_not_seconded, indent=" ", with_age=true, empty="No old proposals this time.")}}
- Pending FCP requests (check your boxes!)
{{-issues::render(issues=in_pre_fcp, indent=" ", empty="No pending FCP requests this time.")}}
{{-issues_rfcbot::render(issues=in_pre_fcp, indent=" ", empty="No pending FCP requests this time.")}}
- Things in FCP (make sure you're good with it)
Urgau marked this conversation as resolved.
Show resolved Hide resolved
{{-issues::render(issues=in_fcp, indent=" ", empty="No FCP requests this time.")}}
{{-issues_rfcbot::render(issues=in_fcp, indent=" ", empty="No FCP requests this time.")}}
- Accepted MCPs
{{-issues::render(issues=mcp_accepted, indent=" ", empty="No new accepted proposals this time.")}}
- MCPs blocked on unresolved concerns
Expand Down
Loading