hive-forge: hard-error unresolved --label names; render dependency timeline events
This commit is contained in:
parent
10285ff764
commit
7bad72d354
4 changed files with 134 additions and 20 deletions
|
|
@ -24,8 +24,9 @@ pub struct Args {
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
assignee: Option<String>,
|
assignee: Option<String>,
|
||||||
/// Label name to attach, repeatable (e.g. `--label area/ops --label
|
/// Label name to attach, repeatable (e.g. `--label area/ops --label
|
||||||
/// type/bug`). Same spelling `labels add` accepts. Unknown names are
|
/// type/bug`). Same spelling `labels add` accepts. An unresolved name
|
||||||
/// silently dropped, matching `labels add`'s existing behavior.
|
/// errors out (before the issue is created) rather than silently
|
||||||
|
/// attaching fewer labels than asked for.
|
||||||
#[arg(long = "label")]
|
#[arg(long = "label")]
|
||||||
labels: Vec<String>,
|
labels: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
@ -35,8 +36,8 @@ pub struct Args {
|
||||||
/// Propagates any I/O error from the body input (`--body-file`,
|
/// Propagates any I/O error from the body input (`--body-file`,
|
||||||
/// stdin), any transport error from the Forgejo REST call (network
|
/// stdin), any transport error from the Forgejo REST call (network
|
||||||
/// unreachable, 4xx/5xx response, token missing/invalid, the
|
/// unreachable, 4xx/5xx response, token missing/invalid, the
|
||||||
/// `--label` lookup's own list-labels call), and any I/O error from
|
/// `--label` lookup's own list-labels call), an unresolved `--label`
|
||||||
/// writing the issue URL to stdout.
|
/// name, and any I/O error from writing the issue URL to stdout.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default();
|
let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default();
|
||||||
let (owner, name) = client.owner_repo()?;
|
let (owner, name) = client.owner_repo()?;
|
||||||
|
|
@ -44,7 +45,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
let all = labels::repo_labels(client)?;
|
let all = labels::repo_labels(client)?;
|
||||||
Some(labels::resolve_ids(&all, &args.labels))
|
Some(labels::resolve_ids(&all, &args.labels)?)
|
||||||
};
|
};
|
||||||
let payload = CreateIssueOption {
|
let payload = CreateIssueOption {
|
||||||
assignee: None,
|
assignee: None,
|
||||||
|
|
|
||||||
|
|
@ -46,7 +46,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
bail!("hive-forge labels add: pass at least one label name");
|
bail!("hive-forge labels add: pass at least one label name");
|
||||||
}
|
}
|
||||||
let all = repo_labels(client)?;
|
let all = repo_labels(client)?;
|
||||||
let ids: Vec<serde_json::Value> = resolve_ids(&all, &labels)
|
let ids: Vec<serde_json::Value> = resolve_ids(&all, &labels)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|id| json!(id))
|
.map(|id| json!(id))
|
||||||
.collect();
|
.collect();
|
||||||
|
|
@ -104,11 +104,36 @@ pub(crate) fn repo_labels(client: &Client) -> Result<Vec<Label>> {
|
||||||
Ok(labels)
|
Ok(labels)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Resolve label names to ids, silently dropping any name that doesn't
|
/// Resolve label names to ids, hard-erroring if any name doesn't match an
|
||||||
/// match a repo label (same behavior as `labels add`/`remove` above —
|
/// existing repo label. A typo used to silently produce fewer labels than
|
||||||
/// keeps this one non-fatal-typo policy in one place).
|
/// intended with no signal — not even a nonzero exit code — so callers
|
||||||
pub(crate) fn resolve_ids(all: &[Label], names: &[String]) -> Vec<i64> {
|
/// (`labels add`, `issue-create --label`, `pr-create --label`) had no way
|
||||||
names.iter().filter_map(|n| lookup_id(all, n)).collect()
|
/// to notice without manually diffing what they asked for against what
|
||||||
|
/// landed. The error lists both the exact names that didn't resolve and
|
||||||
|
/// every label actually available on the repo, so it's fixable from the
|
||||||
|
/// error alone without a second round-trip to `labels list`.
|
||||||
|
pub(crate) fn resolve_ids(all: &[Label], names: &[String]) -> Result<Vec<i64>> {
|
||||||
|
let mut ids = Vec::with_capacity(names.len());
|
||||||
|
let mut unresolved = Vec::new();
|
||||||
|
for n in names {
|
||||||
|
match lookup_id(all, n) {
|
||||||
|
Some(id) => ids.push(id),
|
||||||
|
None => unresolved.push(n.as_str()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !unresolved.is_empty() {
|
||||||
|
let available: Vec<&str> = all.iter().filter_map(|l| l.name.as_deref()).collect();
|
||||||
|
bail!(
|
||||||
|
"unresolved label name(s): {} — available labels: {}",
|
||||||
|
unresolved.join(", "),
|
||||||
|
if available.is_empty() {
|
||||||
|
"(none)".to_owned()
|
||||||
|
} else {
|
||||||
|
available.join(", ")
|
||||||
|
}
|
||||||
|
);
|
||||||
|
}
|
||||||
|
Ok(ids)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn lookup_id(all: &[Label], name: &str) -> Option<i64> {
|
fn lookup_id(all: &[Label], name: &str) -> Option<i64> {
|
||||||
|
|
@ -121,3 +146,44 @@ fn print_label_names(labels: &[Label]) {
|
||||||
let names: Vec<&str> = labels.iter().filter_map(|l| l.name.as_deref()).collect();
|
let names: Vec<&str> = labels.iter().filter_map(|l| l.name.as_deref()).collect();
|
||||||
let _ = print_json(&json!(names));
|
let _ = print_json(&json!(names));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::resolve_ids;
|
||||||
|
use forgejo_api::structs::Label;
|
||||||
|
|
||||||
|
fn label(id: i64, name: &str) -> Label {
|
||||||
|
Label {
|
||||||
|
color: Some(String::new()),
|
||||||
|
description: Some(String::new()),
|
||||||
|
exclusive: None,
|
||||||
|
id: Some(id),
|
||||||
|
is_archived: None,
|
||||||
|
name: Some(name.to_owned()),
|
||||||
|
url: None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn all_names_resolve_returns_ids_in_order() {
|
||||||
|
let all = vec![label(1, "area/ops"), label(2, "type/bug")];
|
||||||
|
let ids = resolve_ids(&all, &["type/bug".to_owned(), "area/ops".to_owned()]).unwrap();
|
||||||
|
assert_eq!(ids, vec![2, 1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unresolved_name_errors_listing_the_typo_and_available_labels() {
|
||||||
|
let all = vec![label(1, "area/ops"), label(2, "type/bug")];
|
||||||
|
let err = resolve_ids(&all, &["area/op".to_owned()]).unwrap_err();
|
||||||
|
let msg = err.to_string();
|
||||||
|
assert!(msg.contains("area/op"), "missing typo'd name: {msg}");
|
||||||
|
assert!(msg.contains("area/ops"), "missing available label: {msg}");
|
||||||
|
assert!(msg.contains("type/bug"), "missing available label: {msg}");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unresolved_name_on_empty_repo_says_none_available() {
|
||||||
|
let err = resolve_ids(&[], &["anything".to_owned()]).unwrap_err();
|
||||||
|
assert!(err.to_string().contains("(none)"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -69,11 +69,13 @@ pub struct Args {
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
topic: Option<String>,
|
topic: Option<String>,
|
||||||
/// Label name to attach, repeatable (e.g. `--label area/ops --label
|
/// Label name to attach, repeatable (e.g. `--label area/ops --label
|
||||||
/// type/bug`). Same spelling `labels add` accepts. In `--agit` mode
|
/// type/bug`). Same spelling `labels add` accepts — an unresolved name
|
||||||
/// this is applied as a follow-up call once the PR number is known
|
/// errors out rather than silently attaching fewer labels than asked
|
||||||
/// (the `AGit` push itself has no label field), so it's silently
|
/// for. In `--agit` mode this is applied as a follow-up call once the
|
||||||
/// skipped if the PR URL couldn't be parsed back out of the push
|
/// PR number is known (the `AGit` push itself has no label field), so
|
||||||
/// output — same fallback as the deferred multi-line body.
|
/// it's silently skipped (not a label-resolution error) if the PR URL
|
||||||
|
/// couldn't be parsed back out of the push output — same fallback as
|
||||||
|
/// the deferred multi-line body.
|
||||||
#[arg(long = "label")]
|
#[arg(long = "label")]
|
||||||
labels: Vec<String>,
|
labels: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
@ -83,8 +85,8 @@ pub struct Args {
|
||||||
/// Propagates any I/O error from the body input (`--body-file`,
|
/// Propagates any I/O error from the body input (`--body-file`,
|
||||||
/// stdin) or the `--push` shellout to git, any transport error from
|
/// stdin) or the `--push` shellout to git, any transport error from
|
||||||
/// the Forgejo REST call (network unreachable, 4xx/5xx response,
|
/// the Forgejo REST call (network unreachable, 4xx/5xx response,
|
||||||
/// token missing/invalid), and any I/O error from writing the PR
|
/// token missing/invalid), an unresolved `--label` name, and any I/O
|
||||||
/// URL to stdout.
|
/// error from writing the PR URL to stdout.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default();
|
let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default();
|
||||||
if args.agit {
|
if args.agit {
|
||||||
|
|
@ -105,7 +107,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
None
|
None
|
||||||
} else {
|
} else {
|
||||||
let all = labels::repo_labels(client)?;
|
let all = labels::repo_labels(client)?;
|
||||||
Some(labels::resolve_ids(&all, &args.labels))
|
Some(labels::resolve_ids(&all, &args.labels)?)
|
||||||
};
|
};
|
||||||
// Note: Forgejo's CreatePullRequestOption has no `draft` /
|
// Note: Forgejo's CreatePullRequestOption has no `draft` /
|
||||||
// `allow_maintainer_edit` fields (verified against the instance's
|
// `allow_maintainer_edit` fields (verified against the instance's
|
||||||
|
|
@ -249,7 +251,7 @@ fn apply_agit_labels(client: &Client, names: &[String], url: &str) -> Result<()>
|
||||||
};
|
};
|
||||||
let (owner, name) = client.owner_repo()?;
|
let (owner, name) = client.owner_repo()?;
|
||||||
let all = labels::repo_labels(client)?;
|
let all = labels::repo_labels(client)?;
|
||||||
let ids: Vec<serde_json::Value> = labels::resolve_ids(&all, names)
|
let ids: Vec<serde_json::Value> = labels::resolve_ids(&all, names)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|id| json!(id))
|
.map(|id| json!(id))
|
||||||
.collect();
|
.collect();
|
||||||
|
|
|
||||||
|
|
@ -196,6 +196,23 @@ fn format_event(ev: &Value) -> String {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
"comment_ref" | "issue_ref" => "referenced from another issue/PR".to_owned(),
|
"comment_ref" | "issue_ref" => "referenced from another issue/PR".to_owned(),
|
||||||
|
"add_dependency" | "remove_dependency" => {
|
||||||
|
let verb = if event_type == "add_dependency" {
|
||||||
|
"added dependency on"
|
||||||
|
} else {
|
||||||
|
"removed dependency on"
|
||||||
|
};
|
||||||
|
let dep = ev.get("dependent_issue");
|
||||||
|
let number = dep
|
||||||
|
.and_then(|d| d.get("number"))
|
||||||
|
.and_then(Value::as_u64)
|
||||||
|
.map_or_else(|| "?".to_owned(), |n| n.to_string());
|
||||||
|
let title = dep
|
||||||
|
.and_then(|d| d.get("title"))
|
||||||
|
.and_then(Value::as_str)
|
||||||
|
.unwrap_or("?");
|
||||||
|
format!("{verb} #{number} ({title})")
|
||||||
|
}
|
||||||
"changed_target_branch" => "changed target branch".to_owned(),
|
"changed_target_branch" => "changed target branch".to_owned(),
|
||||||
"review" => "submitted a review".to_owned(),
|
"review" => "submitted a review".to_owned(),
|
||||||
"lock" => "locked the conversation".to_owned(),
|
"lock" => "locked the conversation".to_owned(),
|
||||||
|
|
@ -344,6 +361,34 @@ mod tests {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn add_dependency_renders_target_issue() {
|
||||||
|
let ev = serde_json::json!({
|
||||||
|
"type": "add_dependency",
|
||||||
|
"user": { "login": "mara" },
|
||||||
|
"created_at": "2026-05-31T12:00:00Z",
|
||||||
|
"dependent_issue": { "number": 2850, "title": "extract swarm controller" },
|
||||||
|
});
|
||||||
|
assert_eq!(
|
||||||
|
format_event(&ev),
|
||||||
|
"**mara @ 2026-05-31T12:00:00Z**: added dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn remove_dependency_renders_target_issue() {
|
||||||
|
let ev = serde_json::json!({
|
||||||
|
"type": "remove_dependency",
|
||||||
|
"user": { "login": "mara" },
|
||||||
|
"created_at": "2026-05-31T12:00:00Z",
|
||||||
|
"dependent_issue": { "number": 2850, "title": "extract swarm controller" },
|
||||||
|
});
|
||||||
|
assert_eq!(
|
||||||
|
format_event(&ev),
|
||||||
|
"**mara @ 2026-05-31T12:00:00Z**: removed dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn unknown_event_type_renders_bracketed_placeholder() {
|
fn unknown_event_type_renders_bracketed_placeholder() {
|
||||||
// Future-proofing: a forge schema bump that adds a new event
|
// Future-proofing: a forge schema bump that adds a new event
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue