mara/hyperhive
mara/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Activity

hive-forge: pr reviews names the poster of an inline comment that is not the reviewer's

Browse source
This commit is contained in:
damocles 2026-11-05 09:31:15 +01:00 • committed by mara
commit c8c5a19761
3 changed files with 53 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

2
docs/tools/forge.md
View file

@ -77,7 +77,7 @@ hive-forge pr edit 42 --base main # retarget a stacked PR onto main
hive-forge pr 42 # PR metadata as JSON
hive-forge pr create --title "..." --head my-branch --push # also `git push forge my-branch`
hive-forge pr create --title "..." --head my-branch --label area/ops # repeatable, same as issue create
hive-forge pr reviews 42 # list reviews; each inline comment as `[#id path:line] text`, tagged (resolved by X) / (no position)
hive-forge pr reviews 42 # list reviews; each inline comment as `[#id path:line] text`, tagged (by X) when its poster is not the reviewer / (resolved by X) / (no position)
hive-forge pr reviews 42 --only unresolved # every inline comment nobody has resolved yet
hive-forge pr timeline 42 # review rows list their line comments (id, path:line, state) too
hive-forge pr reviews 42 --approve # submit APPROVED review

58
hive-forge/src/verbs/pr_reviews.rs
View file

@ -167,6 +167,13 @@ fn resolved_by(c: &Value) -> Option<&str> {
.and_then(Value::as_str)
}
/// Login of whoever posted the comment, if the forge says.
fn posted_by(c: &Value) -> Option<&str> {
c.get("user")
.and_then(|u| u.get("login"))
.and_then(Value::as_str)
}
/// Nobody has resolved the conversation yet. Position plays no part: a
/// comment without one may be a live finding, and a filter built for a
/// re-review gate must not hide it.
@ -175,9 +182,14 @@ fn is_unresolved(c: &Value) -> bool {
}
/// One inline comment as text: `[#id path:line] body`, continuation lines
/// indented, `(resolved by X)` / `(no position: ...)` after the body. Shared by
/// `pr reviews` and `pr timeline`.
pub(crate) fn render_inline_comment(c: &Value) -> String {
/// indented, `(by X)` / `(resolved by X)` / `(no position: ...)` after the body.
/// Shared by `pr reviews` and `pr timeline`.
///
/// A reply is stored in the review of the comment it answers, so under
/// `reviewer`'s heading it can be someone else's: `(by X)` names the poster
/// when it differs from `reviewer`. `pr timeline` shows who commented on its
/// own and passes `None`.
pub(crate) fn render_inline_comment(c: &Value, reviewer: Option<&str>) -> String {
let id = c.get("id").and_then(Value::as_i64).unwrap_or(0);
let body = c
.get("body")
@ -186,6 +198,11 @@ pub(crate) fn render_inline_comment(c: &Value) -> String {
.trim()
.replace('\n', "\n ");
let mut out = format!("[#{id} {}] {body}", comment_location(c));
if let (Some(poster), Some(reviewer)) = (posted_by(c), reviewer)
&& poster != reviewer
{
write!(out, " (by {poster})").ok();
}
if let Some(who) = resolved_by(c) {
write!(out, " (resolved by {who})").ok();
}
@ -232,6 +249,7 @@ fn list_reviews_json(
"path": c.get("path"),
"line": c.get("position"),
"body": c.get("body"),
"user": posted_by(c),
"resolved_by": resolved_by(c),
"no_position": has_no_position(c),
})
@ -296,7 +314,7 @@ fn list_reviews_text(client: &Client, number: u64, reviews: &[Value], only: Opti
println!("{body}");
}
for c in &inline {
println!(" {}", render_inline_comment(c));
println!(" {}", render_inline_comment(c, Some(user)));
}
println!();
}
@ -365,15 +383,15 @@ mod tests {
#[test]
fn render_shows_id_location_body_and_state_tags() {
assert_eq!(
render_inline_comment(&comment(7, 42, None)),
render_inline_comment(&comment(7, 42, None), None),
"[#7 src/a.rs:42] off by one"
);
assert_eq!(
render_inline_comment(&comment(7, 42, Some("mara"))),
render_inline_comment(&comment(7, 42, Some("mara")), None),
"[#7 src/a.rs:42] off by one (resolved by mara)"
);
assert_eq!(
render_inline_comment(&comment(7, 0, None)),
render_inline_comment(&comment(7, 0, None), None),
"[#7 src/a.rs] off by one (no position: outdated or file-level)"
);
}
@ -381,6 +399,30 @@ mod tests {
#[test]
fn render_indents_continuation_lines() {
let c = json!({"id": 1, "path": "f.rs", "position": 2, "body": "one\ntwo"});
assert_eq!(render_inline_comment(&c), "[#1 f.rs:2] one\n two");
assert_eq!(render_inline_comment(&c, None), "[#1 f.rs:2] one\n two");
}
#[test]
fn a_reply_by_someone_else_names_its_poster_under_the_reviewers_heading() {
let mut c = comment(7, 42, None);
c["user"] = json!({"login": "damocles"});
assert_eq!(
render_inline_comment(&c, Some("mara")),
"[#7 src/a.rs:42] off by one (by damocles)"
);
// The reviewer's own comment, a comment with no poster, and a caller
// that shows the poster elsewhere get no tag.
assert_eq!(
render_inline_comment(&c, Some("damocles")),
"[#7 src/a.rs:42] off by one"
);
assert_eq!(
render_inline_comment(&comment(7, 42, None), Some("mara")),
"[#7 src/a.rs:42] off by one"
);
assert_eq!(
render_inline_comment(&c, None),
"[#7 src/a.rs:42] off by one"
);
}
}

4
hive-forge/src/verbs/timeline.rs
View file

@ -494,7 +494,7 @@ fn format_event(ev: &Value) -> String {
let inline = ev.get("inline_comments").and_then(Value::as_array);
for c in inline.into_iter().flatten() {
out.push_str("\n ");
out.push_str(&render_inline_comment(c));
out.push_str(&render_inline_comment(c, None));
}
out
}
@ -503,7 +503,7 @@ fn format_event(ev: &Value) -> String {
"code" => {
let body = ev.get("body").and_then(Value::as_str).unwrap_or("");
match ev.get("inline") {
Some(c) => format!("commented on a line\n {}", render_inline_comment(c)),
Some(c) => format!("commented on a line\n {}", render_inline_comment(c, None)),
None => format!("commented on a line: {body}"),
}
}