hive-forge: don't assert 'conflicts' from a bare mergeable=false
This commit is contained in:
parent
145a20eae6
commit
dd5ccb5ce7
2 changed files with 27 additions and 2 deletions
|
|
@ -127,8 +127,18 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
fn check_ready(client: &Client, repo: &str, number: u64, pull: &PullRequest) -> Result<()> {
|
fn check_ready(client: &Client, repo: &str, number: u64, pull: &PullRequest) -> Result<()> {
|
||||||
match pull.mergeable {
|
match pull.mergeable {
|
||||||
Some(true) => {}
|
Some(true) => {}
|
||||||
|
// Forgejo's `mergeable` is one boolean covering several distinct
|
||||||
|
// causes (draft, a real conflict, a blocked required check, ...) —
|
||||||
|
// asserting "conflicts" and telling the caller to rebase is only
|
||||||
|
// correct for one of those causes and actively wrong advice for the
|
||||||
|
// others (rebasing a draft does nothing). Only name the
|
||||||
|
// cause when the API actually confirms it (draft); otherwise report
|
||||||
|
// the bare observation and let `pr-status` be consulted for detail.
|
||||||
|
Some(false) if pull.draft.unwrap_or(false) => bail!(
|
||||||
|
"pr-merge: PR #{number} is a draft, not mergeable. Mark it ready for review, or pass --force."
|
||||||
|
),
|
||||||
Some(false) => bail!(
|
Some(false) => bail!(
|
||||||
"pr-merge: PR #{number} is not mergeable (conflicts). Rebase it, or pass --force."
|
"pr-merge: PR #{number} is not mergeable. Check `pr-status --pr {number}` for detail, or pass --force."
|
||||||
),
|
),
|
||||||
None => bail!(
|
None => bail!(
|
||||||
"pr-merge: PR #{number} mergeability is still being computed. Retry shortly, or pass --force."
|
"pr-merge: PR #{number} mergeability is still being computed. Retry shortly, or pass --force."
|
||||||
|
|
|
||||||
|
|
@ -107,6 +107,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
let merged = pull.merged.unwrap_or(false);
|
let merged = pull.merged.unwrap_or(false);
|
||||||
// `mergeable` is `null` while the forge is still computing it.
|
// `mergeable` is `null` while the forge is still computing it.
|
||||||
let mergeable = pull.mergeable;
|
let mergeable = pull.mergeable;
|
||||||
|
let draft = pull.draft.unwrap_or(false);
|
||||||
let sha = pull
|
let sha = pull
|
||||||
.head
|
.head
|
||||||
.as_ref()
|
.as_ref()
|
||||||
|
|
@ -132,6 +133,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
"state": state,
|
"state": state,
|
||||||
"merged": merged,
|
"merged": merged,
|
||||||
"mergeable": mergeable,
|
"mergeable": mergeable,
|
||||||
|
"draft": draft,
|
||||||
"head_sha": sha,
|
"head_sha": sha,
|
||||||
"ci_state": ci_state,
|
"ci_state": ci_state,
|
||||||
"ci_statuses": ci_statuses,
|
"ci_statuses": ci_statuses,
|
||||||
|
|
@ -154,6 +156,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> {
|
||||||
state,
|
state,
|
||||||
merged,
|
merged,
|
||||||
mergeable,
|
mergeable,
|
||||||
|
draft,
|
||||||
&sha,
|
&sha,
|
||||||
&ci_state,
|
&ci_state,
|
||||||
&ci_statuses,
|
&ci_statuses,
|
||||||
|
|
@ -304,6 +307,7 @@ fn print_pr(
|
||||||
state: &str,
|
state: &str,
|
||||||
merged: bool,
|
merged: bool,
|
||||||
mergeable: Option<bool>,
|
mergeable: Option<bool>,
|
||||||
|
draft: bool,
|
||||||
sha: &str,
|
sha: &str,
|
||||||
ci_state: &str,
|
ci_state: &str,
|
||||||
ci_statuses: &[Value],
|
ci_statuses: &[Value],
|
||||||
|
|
@ -315,14 +319,25 @@ fn print_pr(
|
||||||
let state_line = if merged {
|
let state_line = if merged {
|
||||||
"merged".to_owned()
|
"merged".to_owned()
|
||||||
} else {
|
} else {
|
||||||
|
// Forgejo's `mergeable` is a single boolean folding together several
|
||||||
|
// distinct causes (draft, real conflicts, blocked required checks,
|
||||||
|
// ...) — printing a specific cause we didn't actually observe is a
|
||||||
|
// wrong diagnosis in a health view, worse than no diagnosis at all.
|
||||||
|
// Only name a cause we can actually confirm from the API response;
|
||||||
|
// otherwise report the bare observation.
|
||||||
let m = match mergeable {
|
let m = match mergeable {
|
||||||
Some(true) => "mergeable: yes",
|
Some(true) => "mergeable: yes",
|
||||||
Some(false) => "mergeable: NO (conflicts)",
|
Some(false) => "mergeable: NO",
|
||||||
None => "mergeable: computing…",
|
None => "mergeable: computing…",
|
||||||
};
|
};
|
||||||
format!("{state} ({m})")
|
format!("{state} ({m})")
|
||||||
};
|
};
|
||||||
println!(" state: {state_line}");
|
println!(" state: {state_line}");
|
||||||
|
// Surfaced as its own line rather than folded into the mergeable text
|
||||||
|
// above — it's a real, confirmed cause (unlike the ones we can't name),
|
||||||
|
// and it's currently invisible unless you happen to notice a `WIP:`
|
||||||
|
// title prefix.
|
||||||
|
println!(" draft: {}", if draft { "yes" } else { "no" });
|
||||||
|
|
||||||
print!(" CI: ");
|
print!(" CI: ");
|
||||||
print_ci(sha, ci_state, ci_statuses);
|
print_ci(sha, ci_state, ci_statuses);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue