hive-forge list: show dep-progress count for items with open dependencies
This commit is contained in:
parent
ddccdf2bab
commit
a796c24037
2 changed files with 77 additions and 10 deletions
|
|
@ -15,6 +15,14 @@
|
||||||
//! matches it against title **and body** on this same endpoint, and a
|
//! matches it against title **and body** on this same endpoint, and a
|
||||||
//! text match is only useful *composed* with the other filters — a
|
//! text match is only useful *composed* with the other filters — a
|
||||||
//! separate verb would have to grow every one of them back.
|
//! separate verb would have to grow every one of them back.
|
||||||
|
//!
|
||||||
|
//! Each row in the pretty (non-`--json`) output also carries a
|
||||||
|
//! `(N/M deps done)` suffix when the item has dependencies and at least
|
||||||
|
//! one is still open — omitted entirely once every dependency is closed
|
||||||
|
//! or there are none, so a ready item's row looks exactly like it did
|
||||||
|
//! before this existed. Forgejo has no server-side dependency filter on
|
||||||
|
//! this endpoint, so this costs one extra request per row shown (not
|
||||||
|
//! per match — only the current page).
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use clap::{Args as ClapArgs, ValueEnum};
|
use clap::{Args as ClapArgs, ValueEnum};
|
||||||
|
|
@ -24,7 +32,7 @@ use forgejo_api::structs::{
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use crate::client::Client;
|
use crate::client::Client;
|
||||||
use crate::verbs::{labels, milestone, print_json};
|
use crate::verbs::{dependency_summaries, labels, milestone, print_json};
|
||||||
|
|
||||||
/// What kind of items to return. Maps onto Forgejo's `type` query
|
/// What kind of items to return. Maps onto Forgejo's `type` query
|
||||||
/// parameter: `issues` / `pulls`, or no filter at all for `both`
|
/// parameter: `issues` / `pulls`, or no filter at all for `both`
|
||||||
|
|
@ -170,7 +178,12 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
return print_json(&items);
|
return print_json(&items);
|
||||||
}
|
}
|
||||||
for item in items.as_array().into_iter().flatten() {
|
for item in items.as_array().into_iter().flatten() {
|
||||||
print_row(item);
|
let progress = item
|
||||||
|
.get("number")
|
||||||
|
.and_then(Value::as_u64)
|
||||||
|
.map(|n| dependency_summaries(client, owner, name, n))
|
||||||
|
.transpose()?;
|
||||||
|
print_row(item, progress.as_deref().and_then(dep_progress));
|
||||||
}
|
}
|
||||||
if let Some(msg) = trailer(
|
if let Some(msg) = trailer(
|
||||||
headers.x_total_count.and_then(|t| u64::try_from(t).ok()),
|
headers.x_total_count.and_then(|t| u64::try_from(t).ok()),
|
||||||
|
|
@ -229,11 +242,12 @@ fn trailer(total: Option<u64>, count: u64, page: u64, limit: u64) -> Option<Stri
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Render one issue/PR as a single `#NNN [author] title` line.
|
/// Render one issue/PR as a single `#NNN [author] title` line, plus a
|
||||||
|
/// `(N/M deps done)` suffix when `progress` is `Some`.
|
||||||
/// Defensive: missing fields drop to placeholders so a partial
|
/// Defensive: missing fields drop to placeholders so a partial
|
||||||
/// response from a future API change still produces readable output
|
/// response from a future API change still produces readable output
|
||||||
/// instead of panicking on `unwrap`.
|
/// instead of panicking on `unwrap`.
|
||||||
fn print_row(item: &Value) {
|
fn print_row(item: &Value, progress: Option<(usize, usize)>) {
|
||||||
let number = item.get("number").and_then(Value::as_u64).unwrap_or(0);
|
let number = item.get("number").and_then(Value::as_u64).unwrap_or(0);
|
||||||
let title = item.get("title").and_then(Value::as_str).unwrap_or("");
|
let title = item.get("title").and_then(Value::as_str).unwrap_or("");
|
||||||
let author = item
|
let author = item
|
||||||
|
|
@ -247,13 +261,63 @@ fn print_row(item: &Value) {
|
||||||
// (which would render every issue as a PR).
|
// (which would render every issue as a PR).
|
||||||
let is_pr = item.get("pull_request").is_some_and(|v| !v.is_null());
|
let is_pr = item.get("pull_request").is_some_and(|v| !v.is_null());
|
||||||
let kind = if is_pr { "PR" } else { " " };
|
let kind = if is_pr { "PR" } else { " " };
|
||||||
println!("#{number:>4} {kind} [{author}] {title}");
|
match progress {
|
||||||
|
Some((done, total)) => {
|
||||||
|
println!("#{number:>4} {kind} [{author}] {title} ({done}/{total} deps done)");
|
||||||
|
}
|
||||||
|
None => println!("#{number:>4} {kind} [{author}] {title}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Dependency completion progress from a `dependency_summaries` list —
|
||||||
|
/// `Some((done, total))` only when there's at least one dependency AND
|
||||||
|
/// at least one of them is still open. `None` for no dependencies, or
|
||||||
|
/// all of them already closed: the operator asked for the counter to
|
||||||
|
/// appear only "when there are deps that are not done", so a ready
|
||||||
|
/// item's row stays exactly as clean as before this existed.
|
||||||
|
fn dep_progress(deps: &[Value]) -> Option<(usize, usize)> {
|
||||||
|
let total = deps.len();
|
||||||
|
if total == 0 {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let done = deps
|
||||||
|
.iter()
|
||||||
|
.filter(|d| d.get("state").and_then(Value::as_str) == Some("closed"))
|
||||||
|
.count();
|
||||||
|
(done < total).then_some((done, total))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
fn dep(state: &str) -> Value {
|
||||||
|
serde_json::json!({ "number": 1, "title": "x", "state": state })
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dep_progress_none_when_no_dependencies() {
|
||||||
|
assert_eq!(dep_progress(&[]), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dep_progress_none_when_all_dependencies_closed() {
|
||||||
|
let deps = [dep("closed"), dep("closed")];
|
||||||
|
assert_eq!(dep_progress(&deps), None);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dep_progress_some_when_at_least_one_dependency_still_open() {
|
||||||
|
let deps = [dep("closed"), dep("open"), dep("closed")];
|
||||||
|
assert_eq!(dep_progress(&deps), Some((2, 3)));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dep_progress_counts_all_open_as_zero_done() {
|
||||||
|
let deps = [dep("open"), dep("open")];
|
||||||
|
assert_eq!(dep_progress(&deps), Some((0, 2)));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn kind_query_types_match_forgejo_enum() {
|
fn kind_query_types_match_forgejo_enum() {
|
||||||
// The forge accepts only `issues` / `pulls` for the `type`
|
// The forge accepts only `issues` / `pulls` for the `type`
|
||||||
|
|
|
||||||
|
|
@ -351,12 +351,15 @@ pub(crate) fn latest_reviews(client: &Client, repo: &str, pr: u64) -> Result<Vec
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The current dependency list for an issue or PR — each entry names
|
/// The current dependency list for an issue or PR — each entry names
|
||||||
/// another issue/PR this one is blocked on. Forgejo's dependency endpoint
|
/// another issue/PR this one is blocked on, with its `number`/`title`/
|
||||||
/// works on the shared issue/PR index (PRs are issues internally under
|
/// `state`. Forgejo's dependency endpoint works on the shared issue/PR
|
||||||
/// the hood), so `issue show` and `pr show` both call this instead of
|
/// index (PRs are issues internally under the hood), so `issue show`,
|
||||||
/// duplicating the fetch-and-shape step. A reviewer asked whether
|
/// `pr show`, and `list`'s dep-progress annotation all call this instead
|
||||||
|
/// of duplicating the fetch-and-shape step. A reviewer asked whether
|
||||||
/// `show`/`view` surface dependencies — they didn't (only `timeline`
|
/// `show`/`view` surface dependencies — they didn't (only `timeline`
|
||||||
/// rendered them, as history); this is the current-state complement.
|
/// rendered them, as history); this is the current-state complement.
|
||||||
|
/// `state` was added alongside `list`'s annotation so a caller can tell
|
||||||
|
/// open deps from closed ones without a second fetch.
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
|
|
@ -373,7 +376,7 @@ pub(crate) fn dependency_summaries(
|
||||||
.send()?;
|
.send()?;
|
||||||
Ok(deps
|
Ok(deps
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|d| json!({ "number": d.number, "title": d.title }))
|
.map(|d| json!({ "number": d.number, "title": d.title, "state": d.state }))
|
||||||
.collect())
|
.collect())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue