From b0bbe4c728ea7994beb8019c41086d5c61c9f653 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 23 Jan 2025 09:13:20 +0100 Subject: [PATCH 1/4] feat: do not assume that the base ref is `main` We still assume that the ultimate trunk is `main`, but this PR means that we can safely merge branches with different bases than `main`. --- src/main.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main.rs b/src/main.rs index 740d96a..73ac255 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,7 +19,7 @@ struct Args { /// How long to wait (seconds) between push attempts. /// - /// This program will retry the final push of `main` exactly once, + /// This program will retry the final push of to the base exactly once, /// after this interval, in order to ensure that github has the chance /// to synchronize itself. #[arg(short = 'i', long, default_value_t = 2.5)] @@ -57,6 +57,7 @@ impl StatusCheck { #[derive(Debug, serde::Deserialize)] #[serde(rename_all = "camelCase")] struct Status { + base_ref_name: String, review_decision: String, status_check_rollup: Vec, } @@ -102,7 +103,7 @@ fn main() -> Result<()> { // get review and current ci status let status = cmd!( sh, - "gh pr view {branch} --json reviewDecision,statusCheckRollup" + "gh pr view {branch} --json baseRefName,reviewDecision,statusCheckRollup" ) .quiet() .read() @@ -125,12 +126,13 @@ fn main() -> Result<()> { } } - // ensure that the branch is at the tip of origin/main for a linear history + // ensure that the branch is at the tip of its base for a linear history + let base = status.base_ref_name; cmd!(sh, "git fetch").run().context("git fetch")?; cmd!(sh, "git checkout {branch}") .run() .context("git checkout branch")?; - let rebase_result = cmd!(sh, "git rebase origin/main").run(); + let rebase_result = cmd!(sh, "git rebase origin/{base}").run(); if rebase_result.is_err() { cmd!(sh, "git rebase --abort") .run() @@ -153,12 +155,12 @@ fn main() -> Result<()> { } // we can now actually merge this to main without breaking anything - cmd!(sh, "git checkout main") + cmd!(sh, "git checkout {base}") .run() - .context("checking out main")?; + .context("checking out base")?; cmd!(sh, "git merge {branch} --ff-only") .run() - .context("performing ff-only merge to main")?; + .context("performing ff-only merge to base")?; // in principle we can now just push; github has some magic to ensure that if you are pushing main // to a commit which is at the tip of an approved pr, then it counts it as a manual merge operation @@ -172,7 +174,7 @@ fn main() -> Result<()> { std::thread::sleep(std::time::Duration::from_secs_f64(args.push_retry_interval)); cmd!(sh, "git push") .run() - .context("2nd attempt to push to main")?; + .context("2nd attempt to push to base")?; } Ok(()) From a8abef09da60ae6bdfd9418c0f1fd90a78786a19 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 23 Jan 2025 09:25:56 +0100 Subject: [PATCH 2/4] fix: status check data model --- src/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 73ac255..82b88be 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,7 +40,7 @@ struct StatusCheck { #[serde(rename = "__typename")] type_name: String, name: String, - status: String, + status: Option, conclusion: String, } @@ -50,7 +50,8 @@ impl StatusCheck { } fn is_successy(&self) -> bool { - self.status == "COMPLETED" && (self.conclusion == "SUCCESS" || self.conclusion == "SKIPPED") + self.status.as_deref() == Some("COMPLETED") + && (self.conclusion == "SUCCESS" || self.conclusion == "SKIPPED") } } From 65e41f5b561608a38102a749c01f5d989a7fdb29 Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 23 Jan 2025 13:39:29 +0100 Subject: [PATCH 3/4] fix: correctly filter status checks --- Cargo.lock | 16 ---------------- Cargo.toml | 1 - src/main.rs | 42 +++++++++++++++++++++++++++++------------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0e0c681..39b0f6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -104,12 +104,6 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b63caa9aa9397e2d9480a9b13673856c78d8ac123288526c37d7839f2a86990" -[[package]] -name = "either" -version = "1.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" - [[package]] name = "heck" version = "0.5.0" @@ -122,15 +116,6 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7943c866cc5cd64cbc25b2e01621d07fa8eb2a1a23160ee81ce38704e97b8ecf" -[[package]] -name = "itertools" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b192c782037fadd9cfa75548310488aabdbf3d2da73885b31bd0abd03351285" -dependencies = [ - "either", -] - [[package]] name = "itoa" version = "1.0.14" @@ -149,7 +134,6 @@ version = "0.1.1" dependencies = [ "anyhow", "clap", - "itertools", "serde", "serde_json", "xshell", diff --git a/Cargo.toml b/Cargo.toml index a646720..c006782 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ dist = true [dependencies] anyhow = "1.0.95" clap = { version = "4.5.27", features = ["derive"] } -itertools = "0.14.0" serde = { version = "1.0.217", features = ["derive"] } serde_json = "1.0.137" xshell = "0.2.7" diff --git a/src/main.rs b/src/main.rs index 82b88be..b22567d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,5 @@ use anyhow::{anyhow, bail, Context, Result}; use clap::Parser; -use itertools::Itertools as _; use serde_json::Value; use xshell::{cmd, Shell}; @@ -36,25 +35,39 @@ fn ensure_tool(sh: &Shell, tool_name: &str) -> Result<()> { #[derive(Debug, serde::Deserialize)] #[serde(rename_all = "camelCase")] -struct StatusCheck { - #[serde(rename = "__typename")] - type_name: String, +struct CheckRun { + // we do want to deserialize the name + #[allow(dead_code)] name: String, status: Option, conclusion: String, } -impl StatusCheck { - fn is_check_run(&self) -> bool { - self.type_name == "CheckRun" - } - +impl CheckRun { fn is_successy(&self) -> bool { self.status.as_deref() == Some("COMPLETED") && (self.conclusion == "SUCCESS" || self.conclusion == "SKIPPED") } } +#[derive(Debug, serde::Deserialize)] +#[serde(tag = "__typename")] +enum StatusCheck { + CheckRun(CheckRun), + // we don't care about the value here, but serde needs to know to deserialize _something_ + #[allow(dead_code)] + StatusContext(Value), +} + +impl StatusCheck { + fn as_check_run(&self) -> Option<&CheckRun> { + match self { + Self::CheckRun(check_run) => Some(check_run), + _ => None, + } + } +} + #[derive(Debug, serde::Deserialize)] #[serde(rename_all = "camelCase")] struct Status { @@ -119,11 +132,14 @@ fn main() -> Result<()> { let non_success = status .status_check_rollup .iter() - .filter(|check| check.is_check_run() && !check.is_successy()) - .map(|check| &check.name) - .join(", "); + .filter_map(StatusCheck::as_check_run) + .filter(|check_run| !check_run.is_successy()) + .collect::>(); if !non_success.is_empty() { - bail!("some ci checks are incomplete or unsuccessful: {non_success}"); + for check_run in non_success { + println!("{check_run:?}"); + } + bail!("some ci checks are incomplete or unsuccessful"); } } From 030a32fe0c67d0807e7893a885aa17684b8694ba Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Thu, 23 Jan 2025 13:40:07 +0100 Subject: [PATCH 4/4] feat: dry-run flag --- src/main.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main.rs b/src/main.rs index b22567d..ce27a94 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,6 +23,10 @@ struct Args { /// to synchronize itself. #[arg(short = 'i', long, default_value_t = 2.5)] push_retry_interval: f64, + + /// When set, perform checks but do not actually change the repo state. + #[arg(short, long)] + dry_run: bool, } fn ensure_tool(sh: &Shell, tool_name: &str) -> Result<()> { @@ -143,6 +147,11 @@ fn main() -> Result<()> { } } + if args.dry_run { + println!("all checks OK but aborting due to dry run"); + return Ok(()); + } + // ensure that the branch is at the tip of its base for a linear history let base = status.base_ref_name; cmd!(sh, "git fetch").run().context("git fetch")?;