refactor(hive-forge): share latest-per-reviewer logic between pr-status and pr-merge
pr-status and pr-merge both computed 'latest non-comment review verdict per reviewer' independently (identical oldest-first, COMMENT/PENDING-skipping, supersede-by-later loop). Extract it to a shared verbs::latest_reviews helper so the verdict semantics live in one place and can't drift between the health view and the pre-merge changes-requested gate. Pure dedup, no behaviour change.
This commit is contained in:
parent
f6e1651dff
commit
f633abbdc4
3 changed files with 42 additions and 53 deletions
|
|
@ -36,6 +36,8 @@ pub mod view;
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
|
use crate::client::Client;
|
||||||
|
|
||||||
/// Pretty-print a `serde_json` value to stdout with a trailing newline,
|
/// Pretty-print a `serde_json` value to stdout with a trailing newline,
|
||||||
/// matching the bash script's `| jq` output shape.
|
/// matching the bash script's `| jq` output shape.
|
||||||
pub(crate) fn print_json(v: &Value) -> Result<()> {
|
pub(crate) fn print_json(v: &Value) -> Result<()> {
|
||||||
|
|
@ -43,3 +45,38 @@ pub(crate) fn print_json(v: &Value) -> Result<()> {
|
||||||
println!("{s}");
|
println!("{s}");
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Latest non-comment review verdict per reviewer on a PR, as
|
||||||
|
/// `(login, state)`. Reviews come oldest-first, so a later verdict from
|
||||||
|
/// the same user supersedes an earlier one; `COMMENT` / `PENDING`
|
||||||
|
/// reviews carry no verdict and are skipped. Shared by `pr-status` (for
|
||||||
|
/// its health view + readiness verdict) and `pr-merge` (for its
|
||||||
|
/// pre-merge changes-requested gate) so the verdict semantics stay in
|
||||||
|
/// one place.
|
||||||
|
pub(crate) fn latest_reviews(
|
||||||
|
client: &Client,
|
||||||
|
repo: &str,
|
||||||
|
pr: u64,
|
||||||
|
) -> Result<Vec<(String, String)>> {
|
||||||
|
let reviews = client.get_json_all(&format!("/repos/{repo}/pulls/{pr}/reviews"), 10)?;
|
||||||
|
let mut latest: Vec<(String, String)> = Vec::new();
|
||||||
|
for r in &reviews {
|
||||||
|
let Some(login) = r
|
||||||
|
.get("user")
|
||||||
|
.and_then(|u| u.get("login"))
|
||||||
|
.and_then(Value::as_str)
|
||||||
|
else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
let st = r.get("state").and_then(Value::as_str).unwrap_or("");
|
||||||
|
if st == "COMMENT" || st == "PENDING" || st.is_empty() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if let Some(slot) = latest.iter_mut().find(|(l, _)| l == login) {
|
||||||
|
st.clone_into(&mut slot.1);
|
||||||
|
} else {
|
||||||
|
latest.push((login.to_owned(), st.to_owned()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Ok(latest)
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -136,30 +136,10 @@ fn check_ready(client: &Client, repo: &str, number: u64, pull: &Value) -> Result
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Latest verdict per reviewer wins (reviews page oldest-first), so a
|
// Block only on reviewers whose *current* verdict requests changes
|
||||||
// later APPROVED supersedes an earlier REQUEST_CHANGES. Block only on
|
// (latest-per-reviewer, so a later APPROVED clears an earlier
|
||||||
// reviewers whose *current* verdict requests changes.
|
// REQUEST_CHANGES). Shares the verdict logic with `pr-status`.
|
||||||
let reviews = client.get_json_all(&format!("/repos/{repo}/pulls/{number}/reviews"), 10)?;
|
let blockers: Vec<String> = super::latest_reviews(client, repo, number)?
|
||||||
let mut latest: Vec<(String, String)> = Vec::new();
|
|
||||||
for r in &reviews {
|
|
||||||
let Some(login) = r
|
|
||||||
.get("user")
|
|
||||||
.and_then(|u| u.get("login"))
|
|
||||||
.and_then(Value::as_str)
|
|
||||||
else {
|
|
||||||
continue;
|
|
||||||
};
|
|
||||||
let st = r.get("state").and_then(Value::as_str).unwrap_or("");
|
|
||||||
if st == "COMMENT" || st == "PENDING" || st.is_empty() {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
if let Some(slot) = latest.iter_mut().find(|(l, _)| l == login) {
|
|
||||||
st.clone_into(&mut slot.1);
|
|
||||||
} else {
|
|
||||||
latest.push((login.to_owned(), st.to_owned()));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
let blockers: Vec<String> = latest
|
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter(|(_, st)| st == "REQUEST_CHANGES")
|
.filter(|(_, st)| st == "REQUEST_CHANGES")
|
||||||
.map(|(login, _)| login)
|
.map(|(login, _)| login)
|
||||||
|
|
|
||||||
|
|
@ -85,7 +85,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
|
|
||||||
let (ci_state, ci_statuses) = fetch_combined(client, repo, &sha)?;
|
let (ci_state, ci_statuses) = fetch_combined(client, repo, &sha)?;
|
||||||
let reviews = latest_reviews(client, repo, pr)?;
|
let reviews = super::latest_reviews(client, repo, pr)?;
|
||||||
let last = last_comment(client, repo, pr)?;
|
let last = last_comment(client, repo, pr)?;
|
||||||
|
|
||||||
if client.json_mode() {
|
if client.json_mode() {
|
||||||
|
|
@ -151,34 +151,6 @@ fn fetch_combined(client: &Client, repo: &str, sha: &str) -> Result<(String, Vec
|
||||||
Ok((state, statuses))
|
Ok((state, statuses))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Latest non-comment review verdict per reviewer, as `(login, state)`.
|
|
||||||
/// Reviews come oldest-first; a later one supersedes an earlier one from
|
|
||||||
/// the same user. `COMMENT` / `PENDING` reviews carry no verdict and are
|
|
||||||
/// skipped.
|
|
||||||
fn latest_reviews(client: &Client, repo: &str, pr: u64) -> Result<Vec<(String, String)>> {
|
|
||||||
let reviews = client.get_json_all(&format!("/repos/{repo}/pulls/{pr}/reviews"), 10)?;
|
|
||||||
let mut latest: Vec<(String, String)> = Vec::new();
|
|
||||||
for r in &reviews {
|
|
||||||
let Some(login) = r
|
|
||||||
.get("user")
|
|
||||||
.and_then(|u| u.get("login"))
|
|
||||||
.and_then(Value::as_str)
|
|
||||||
else {
|
|
||||||
continue;
|
|
||||||
};
|
|
||||||
let st = r.get("state").and_then(Value::as_str).unwrap_or("");
|
|
||||||
if st == "COMMENT" || st == "PENDING" || st.is_empty() {
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
if let Some(slot) = latest.iter_mut().find(|(l, _)| l == login) {
|
|
||||||
st.clone_into(&mut slot.1);
|
|
||||||
} else {
|
|
||||||
latest.push((login.to_owned(), st.to_owned()));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Ok(latest)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The most recent issue comment on the PR, as `(login, created_at)`.
|
/// The most recent issue comment on the PR, as `(login, created_at)`.
|
||||||
/// Comments page oldest-first; we drain (bounded) and take the max
|
/// Comments page oldest-first; we drain (bounded) and take the max
|
||||||
/// timestamp so a long thread still reports the genuinely-latest one.
|
/// timestamp so a long thread still reports the genuinely-latest one.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue