hive-forge: pr status lists only pending review requests
This commit is contained in:
parent
b53365e371
commit
1ff4a0057e
2 changed files with 43 additions and 2 deletions
|
|
@ -192,7 +192,9 @@ One-stop PR health view (`<n>`, positional — `--pr <n>` also still works):
|
||||||
mergeable state, CI checks, requested reviewers + review verdicts, and the
|
mergeable state, CI checks, requested reviewers + review verdicts, and the
|
||||||
last-comment timestamp — the things you need to know whether a PR is ready
|
last-comment timestamp — the things you need to know whether a PR is ready
|
||||||
to merge (CI must pass before merge). `--sha <sha>` is a CI-only fast path
|
to merge (CI must pass before merge). `--sha <sha>` is a CI-only fast path
|
||||||
for a raw commit.
|
for a raw commit. The `reviewers` line lists pending requests only: a
|
||||||
|
reviewer whose verdict still stands is left out even though the forge keeps
|
||||||
|
them in its requested list, while one whose review is stale or dismissed stays.
|
||||||
|
|
||||||
A failing/erroring CI context prints its job link. The process exit code
|
A failing/erroring CI context prints its job link. The process exit code
|
||||||
is a **merge-readiness** verdict for a PR number (0 only when CI is green
|
is a **merge-readiness** verdict for a PR number (0 only when CI is green
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@
|
||||||
use anyhow::{Context, Result, bail};
|
use anyhow::{Context, Result, bail};
|
||||||
use clap::Args as ClapArgs;
|
use clap::Args as ClapArgs;
|
||||||
use forgejo_api::structs::IssueGetCommentsQuery;
|
use forgejo_api::structs::IssueGetCommentsQuery;
|
||||||
|
use hive_types::review::{ReviewInfo, standing};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use crate::client::{Client, index};
|
use crate::client::{Client, index};
|
||||||
|
|
@ -100,6 +101,16 @@ fn sha_status(client: &Client, sha: &str) -> Result<()> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The forge keeps a reviewer who has already answered in `requested_reviewers`,
|
||||||
|
/// so a login only counts as a pending request when it has no review that still
|
||||||
|
/// stands. A stale or dismissed review needs a fresh one, so that reviewer stays.
|
||||||
|
fn pending_requests(listed: Vec<String>, reviews: &[ReviewInfo]) -> Vec<String> {
|
||||||
|
listed
|
||||||
|
.into_iter()
|
||||||
|
.filter(|login| standing(reviews, login).is_none_or(|r| r.state == "REQUEST_REVIEW"))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
/// Full PR health view. Exit code is a merge-readiness verdict.
|
/// Full PR health view. Exit code is a merge-readiness verdict.
|
||||||
fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
let (owner, name) = client.owner_repo()?;
|
let (owner, name) = client.owner_repo()?;
|
||||||
|
|
@ -122,7 +133,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
.and_then(|h| h.sha.clone())
|
.and_then(|h| h.sha.clone())
|
||||||
.with_context(|| format!("pr status: PR #{pr} has no head.sha"))?;
|
.with_context(|| format!("pr status: PR #{pr} has no head.sha"))?;
|
||||||
|
|
||||||
let requested: Vec<String> = pull
|
let listed: Vec<String> = pull
|
||||||
.requested_reviewers
|
.requested_reviewers
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.unwrap_or_default()
|
.unwrap_or_default()
|
||||||
|
|
@ -132,6 +143,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
|
|
||||||
let (ci_state, ci_statuses) = fetch_combined(client, &sha)?;
|
let (ci_state, ci_statuses) = fetch_combined(client, &sha)?;
|
||||||
let reviews = super::latest_reviews(client, repo, pr)?;
|
let reviews = super::latest_reviews(client, repo, pr)?;
|
||||||
|
let requested = pending_requests(listed, &reviews);
|
||||||
let last = last_comment(client, pr)?;
|
let last = last_comment(client, pr)?;
|
||||||
|
|
||||||
if client.json_mode() {
|
if client.json_mode() {
|
||||||
|
|
@ -402,6 +414,33 @@ mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use clap::Parser;
|
use clap::Parser;
|
||||||
|
|
||||||
|
fn info(login: &str, verdict: &str, old_head: bool) -> ReviewInfo {
|
||||||
|
ReviewInfo {
|
||||||
|
login: login.to_owned(),
|
||||||
|
state: verdict.to_owned(),
|
||||||
|
stale: old_head,
|
||||||
|
dismissed: false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A reviewer whose verdict still stands has answered; one with no
|
||||||
|
/// review, a bare re-request, or a verdict on an older head is pending.
|
||||||
|
#[test]
|
||||||
|
fn only_reviewers_without_a_standing_verdict_are_pending() {
|
||||||
|
let reviews = [
|
||||||
|
info("answered", "APPROVED", false),
|
||||||
|
info("rerequested", "REQUEST_REVIEW", false),
|
||||||
|
info("stale", "REQUEST_CHANGES", true),
|
||||||
|
];
|
||||||
|
let listed = ["answered", "rerequested", "stale", "never-reviewed"]
|
||||||
|
.map(String::from)
|
||||||
|
.to_vec();
|
||||||
|
assert_eq!(
|
||||||
|
pending_requests(listed, &reviews),
|
||||||
|
["rerequested", "stale", "never-reviewed"]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// `Args` is a flattened arg group, not a `Parser`, so parsing it
|
/// `Args` is a flattened arg group, not a `Parser`, so parsing it
|
||||||
/// standalone needs a wrapper. Without one these cases could only be
|
/// standalone needs a wrapper. Without one these cases could only be
|
||||||
/// written against the whole binary's clap tree.
|
/// written against the whole binary's clap tree.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue