From 1422f5043bf55ad7df1e890907334558c69c6b26 Mon Sep 17 00:00:00 2001 From: Luis Ball Date: Mon, 29 Dec 2025 07:45:58 -0800 Subject: [PATCH 1/4] feat: add land command for merging stacked PRs Add a new 'land' subcommand that intelligently merges a stack of PRs: - Finds the topmost PR where all PRs below it are approved - Updates that PR's base to the target branch (main/master) - Squash-merges that single PR (contains all commits from the stack) - Closes all PRs below it with a 'Landed via #N' comment Features: - --dry-run: Preview what would happen without making changes - --no-approval: Skip approval requirement check - --count N: Only land bottom N PRs in the stack - Draft PRs block landing of PRs above them This follows the spr/Graphite optimization pattern for efficient stack landing. --- Cargo.lock | 41 +++ Cargo.toml | 1 + src/api/land.rs | 336 ++++++++++++++++++++++++ src/api/mod.rs | 1 + src/api/pull_request.rs | 9 + src/land.rs | 554 ++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 3 +- src/main.rs | 140 +++++++++- 8 files changed, 1083 insertions(+), 2 deletions(-) create mode 100644 src/api/land.rs create mode 100644 src/land.rs diff --git a/Cargo.lock b/Cargo.lock index 1403803..fc1fd85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -436,6 +436,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serial_test", "tokio", "tokio-test", ] @@ -1285,6 +1286,15 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + [[package]] name = "scopeguard" version = "1.2.0" @@ -1301,6 +1311,12 @@ dependencies = [ "untrusted", ] +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "serde" version = "1.0.228" @@ -1354,6 +1370,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b258109f244e1d6891bf1053a55d63a5cd4f8f4c30cf9a1280989f80e7a1fa9" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d69265a08751de7844521fd15003ae0a888e035773ba05695c5c759a6f89eef" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.111", +] + [[package]] name = "shell-words" version = "1.1.1" diff --git a/Cargo.toml b/Cargo.toml index 504a761..3e4f69c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,4 +30,5 @@ chrono = { version = "0.4", features = ["serde"] } [dev-dependencies] insta = "1.39" mockito = "1.4" +serial_test = "3.1" tokio-test = "0.4" diff --git a/src/api/land.rs b/src/api/land.rs new file mode 100644 index 0000000..7c102c3 --- /dev/null +++ b/src/api/land.rs @@ -0,0 +1,336 @@ +//! GitHub API methods for landing PRs +//! +//! This module provides functions to: +//! - Update a PR's base branch +//! - Merge a PR using squash strategy +//! - Close a PR with a comment + +use reqwest::Client; +use serde::{Deserialize, Serialize}; +use std::error::Error; +use std::time::Duration; + +use crate::Credentials; + +/// Request body for updating a PR's base branch +#[derive(Serialize, Debug)] +struct UpdatePrBaseRequest<'a> { + base: &'a str, +} + +/// Request body for merging a PR +#[derive(Serialize, Debug)] +struct MergePrRequest<'a> { + merge_method: &'a str, +} + +/// Request body for closing a PR +#[derive(Serialize, Debug)] +struct ClosePrRequest { + state: &'static str, +} + +/// Request body for adding a comment +#[derive(Serialize, Debug)] +struct AddCommentRequest<'a> { + body: &'a str, +} + +/// Response from merge endpoint +#[derive(Deserialize, Debug)] +#[allow(dead_code)] +struct MergeResponse { + sha: String, + merged: bool, + message: String, +} + +/// Response with HTML URL +#[derive(Deserialize, Debug)] +struct PrResponse { + html_url: String, +} + +fn build_request(client: &Client, credentials: &Credentials, url: &str) -> reqwest::RequestBuilder { + client + .patch(url) + .timeout(Duration::from_secs(30)) + .header("Authorization", format!("token {}", credentials.token)) + .header("User-Agent", "luqven/gh-stack") + .header("Accept", "application/vnd.github.v3+json") +} + +fn build_put_request( + client: &Client, + credentials: &Credentials, + url: &str, +) -> reqwest::RequestBuilder { + client + .put(url) + .timeout(Duration::from_secs(30)) + .header("Authorization", format!("token {}", credentials.token)) + .header("User-Agent", "luqven/gh-stack") + .header("Accept", "application/vnd.github.v3+json") +} + +fn build_post_request( + client: &Client, + credentials: &Credentials, + url: &str, +) -> reqwest::RequestBuilder { + client + .post(url) + .timeout(Duration::from_secs(30)) + .header("Authorization", format!("token {}", credentials.token)) + .header("User-Agent", "luqven/gh-stack") + .header("Accept", "application/vnd.github.v3+json") +} + +/// Update a PR's base branch +/// +/// # Arguments +/// * `pr_number` - The PR number +/// * `new_base` - The new base branch name (e.g., "main") +/// * `repository` - Repository in "owner/repo" format +/// * `credentials` - GitHub credentials +pub async fn update_pr_base( + pr_number: usize, + new_base: &str, + repository: &str, + credentials: &Credentials, +) -> Result<(), Box> { + let client = Client::new(); + let url = format!( + "{}/repos/{}/pulls/{}", + super::github_api_base(), + repository, + pr_number + ); + + let body = UpdatePrBaseRequest { base: new_base }; + let response = build_request(&client, credentials, &url) + .json(&body) + .send() + .await?; + + if !response.status().is_success() { + let status = response.status(); + let text = response.text().await.unwrap_or_default(); + return Err(format!("Failed to update PR base ({}): {}", status, text).into()); + } + + Ok(()) +} + +/// Merge a PR using squash strategy +/// +/// # Arguments +/// * `pr_number` - The PR number +/// * `repository` - Repository in "owner/repo" format +/// * `credentials` - GitHub credentials +/// +/// # Returns +/// The HTML URL of the merged PR +pub async fn merge_pr( + pr_number: usize, + repository: &str, + credentials: &Credentials, +) -> Result> { + let client = Client::new(); + let url = format!( + "{}/repos/{}/pulls/{}/merge", + super::github_api_base(), + repository, + pr_number + ); + + let body = MergePrRequest { + merge_method: "squash", + }; + let response = build_put_request(&client, credentials, &url) + .json(&body) + .send() + .await?; + + if !response.status().is_success() { + let status = response.status(); + let text = response.text().await.unwrap_or_default(); + return Err(format!("Failed to merge PR ({}): {}", status, text).into()); + } + + let merge_response: MergeResponse = response.json().await?; + if !merge_response.merged { + return Err(format!("PR was not merged: {}", merge_response.message).into()); + } + + // Get the PR HTML URL + let pr_url = format!( + "{}/repos/{}/pulls/{}", + super::github_api_base(), + repository, + pr_number + ); + let pr_response = client + .get(&pr_url) + .timeout(Duration::from_secs(10)) + .header("Authorization", format!("token {}", credentials.token)) + .header("User-Agent", "luqven/gh-stack") + .send() + .await?; + + let pr_data: PrResponse = pr_response.json().await?; + Ok(pr_data.html_url) +} + +/// Close a PR with a comment +/// +/// # Arguments +/// * `pr_number` - The PR number +/// * `comment` - Comment to add before closing +/// * `repository` - Repository in "owner/repo" format +/// * `credentials` - GitHub credentials +pub async fn close_pr_with_comment( + pr_number: usize, + comment: &str, + repository: &str, + credentials: &Credentials, +) -> Result<(), Box> { + let client = Client::new(); + + // First, add a comment + let comment_url = format!( + "{}/repos/{}/issues/{}/comments", + super::github_api_base(), + repository, + pr_number + ); + let comment_body = AddCommentRequest { body: comment }; + let comment_response = build_post_request(&client, credentials, &comment_url) + .json(&comment_body) + .send() + .await?; + + if !comment_response.status().is_success() { + let status = comment_response.status(); + let text = comment_response.text().await.unwrap_or_default(); + return Err(format!("Failed to add comment ({}): {}", status, text).into()); + } + + // Then close the PR + let close_url = format!( + "{}/repos/{}/pulls/{}", + super::github_api_base(), + repository, + pr_number + ); + let close_body = ClosePrRequest { state: "closed" }; + let close_response = build_request(&client, credentials, &close_url) + .json(&close_body) + .send() + .await?; + + if !close_response.status().is_success() { + let status = close_response.status(); + let text = close_response.text().await.unwrap_or_default(); + return Err(format!("Failed to close PR ({}): {}", status, text).into()); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use mockito::Server; + use serial_test::serial; + + #[tokio::test] + #[serial] + async fn test_update_pr_base() { + let mut server = Server::new_async().await; + + let mock = server + .mock("PATCH", "/repos/owner/repo/pulls/123") + .match_body(mockito::Matcher::Json(serde_json::json!({"base": "main"}))) + .with_status(200) + .with_body("{}") + .create_async() + .await; + + std::env::set_var("GITHUB_API_BASE", server.url()); + + let creds = Credentials::new("test-token"); + let result = update_pr_base(123, "main", "owner/repo", &creds).await; + + assert!(result.is_ok()); + mock.assert_async().await; + } + + #[tokio::test] + #[serial] + async fn test_merge_pr() { + let mut server = Server::new_async().await; + + let merge_mock = server + .mock("PUT", "/repos/owner/repo/pulls/123/merge") + .match_body(mockito::Matcher::Json( + serde_json::json!({"merge_method": "squash"}), + )) + .with_status(200) + .with_body(r#"{"sha": "abc123", "merged": true, "message": "Pull Request successfully merged"}"#) + .create_async() + .await; + + let pr_mock = server + .mock("GET", "/repos/owner/repo/pulls/123") + .with_status(200) + .with_body(r#"{"html_url": "https://github.com/owner/repo/pull/123"}"#) + .create_async() + .await; + + std::env::set_var("GITHUB_API_BASE", server.url()); + + let creds = Credentials::new("test-token"); + let result = merge_pr(123, "owner/repo", &creds).await; + + assert!(result.is_ok()); + assert_eq!(result.unwrap(), "https://github.com/owner/repo/pull/123"); + merge_mock.assert_async().await; + pr_mock.assert_async().await; + } + + #[tokio::test] + #[serial] + async fn test_close_pr_with_comment() { + let mut server = Server::new_async().await; + + let comment_mock = server + .mock("POST", "/repos/owner/repo/issues/123/comments") + .match_body(mockito::Matcher::Json( + serde_json::json!({"body": "Landed via #456"}), + )) + .with_status(201) + .with_body("{}") + .create_async() + .await; + + let close_mock = server + .mock("PATCH", "/repos/owner/repo/pulls/123") + .match_body(mockito::Matcher::Json( + serde_json::json!({"state": "closed"}), + )) + .with_status(200) + .with_body("{}") + .create_async() + .await; + + std::env::set_var("GITHUB_API_BASE", server.url()); + + let creds = Credentials::new("test-token"); + let result = close_pr_with_comment(123, "Landed via #456", "owner/repo", &creds).await; + + assert!(result.is_ok()); + comment_mock.assert_async().await; + close_mock.assert_async().await; + } +} diff --git a/src/api/mod.rs b/src/api/mod.rs index c5fe9c4..e660514 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -2,6 +2,7 @@ use crate::Credentials; use reqwest::{Client, RequestBuilder}; use std::time::Duration; +pub mod land; pub mod pull_request; pub mod search; diff --git a/src/api/pull_request.rs b/src/api/pull_request.rs index ca6a285..6853408 100644 --- a/src/api/pull_request.rs +++ b/src/api/pull_request.rs @@ -25,6 +25,15 @@ pub struct PullRequestReview { } impl PullRequestReview { + /// Create a new PullRequestReview for testing purposes + #[cfg(test)] + pub fn new_for_test(state: PullRequestReviewState) -> Self { + PullRequestReview { + state, + body: String::new(), + } + } + pub fn is_approved(&self) -> bool { self.state == PullRequestReviewState::APPROVED } diff --git a/src/land.rs b/src/land.rs new file mode 100644 index 0000000..247d8bf --- /dev/null +++ b/src/land.rs @@ -0,0 +1,554 @@ +//! Landing logic for stacked PRs +//! +//! This module implements the spr/Graphite optimization pattern: +//! 1. Find the topmost PR where all PRs below it are approved +//! 2. Update that PR's base to the target branch +//! 3. Squash-merge that single PR (contains all commits from the stack) +//! 4. Close all PRs below it with a comment linking to the merged PR + +use std::error::Error; +use std::fmt; +use std::rc::Rc; + +use crate::api::PullRequest; +use crate::graph::FlatDep; +use crate::Credentials; + +/// Represents a plan for landing a stack of PRs +#[derive(Debug)] +pub struct LandPlan { + /// The PR that will be merged (topmost mergeable PR) + pub top_pr: Rc, + /// PRs below top that will be closed after merge + pub prs_to_close: Vec>, + /// Target branch to merge into (e.g., "main" or "master") + pub target_branch: String, + /// Repository in "owner/repo" format + pub repository: String, +} + +/// Result of a successful landing operation +#[derive(Debug)] +pub struct LandResult { + /// The PR that was merged + pub merged_pr: Rc, + /// PRs that were closed + pub closed_prs: Vec>, + /// URL of the merged PR + pub merge_url: String, +} + +/// Errors that can occur during landing +#[derive(Debug)] +pub enum LandError { + /// No PRs found in the stack + NoPRsInStack, + /// No PRs are in a mergeable state + NoPRsMergeable { reason: String }, + /// A PR is in draft state and blocks landing + DraftBlocking { pr_number: usize }, + /// A PR requires approval + ApprovalRequired { pr_number: usize }, + /// API call failed + ApiError { message: String }, +} + +impl fmt::Display for LandError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + LandError::NoPRsInStack => write!(f, "No PRs found in the stack"), + LandError::NoPRsMergeable { reason } => { + write!(f, "No PRs are mergeable: {}", reason) + } + LandError::DraftBlocking { pr_number } => { + write!( + f, + "PR #{} is a draft and blocks landing of PRs above it", + pr_number + ) + } + LandError::ApprovalRequired { pr_number } => { + write!(f, "PR #{} requires approval", pr_number) + } + LandError::ApiError { message } => write!(f, "API error: {}", message), + } + } +} + +impl Error for LandError {} + +/// Options for creating a land plan +pub struct LandOptions { + /// Whether to require approval on all PRs + pub require_approval: bool, + /// Maximum number of PRs to land (None = all mergeable) + pub max_count: Option, +} + +impl Default for LandOptions { + fn default() -> Self { + LandOptions { + require_approval: true, + max_count: None, + } + } +} + +/// Order the stack from base to top (PRs targeting main/master first) +fn order_stack_base_to_top(stack: &FlatDep) -> Vec> { + // Find root PRs (those with no parent in the stack) + let mut ordered = Vec::new(); + let mut remaining: Vec<_> = stack.iter().collect(); + + // Start with PRs that have no parent (base of stack) + while !remaining.is_empty() { + let mut found_any = false; + + remaining.retain(|(pr, parent)| { + let dominated_by_parent = parent + .as_ref() + .map(|p| { + // Check if parent is already in ordered list + ordered + .iter() + .any(|o: &Rc| o.number() == p.number()) + }) + .unwrap_or(true); // No parent means it's a root + + if dominated_by_parent { + ordered.push(pr.clone()); + found_any = true; + false // Remove from remaining + } else { + true // Keep in remaining + } + }); + + // Safety: prevent infinite loop if graph is malformed + if !found_any && !remaining.is_empty() { + // Add remaining PRs in any order + for (pr, _) in remaining.drain(..) { + ordered.push(pr.clone()); + } + } + } + + ordered +} + +/// Check if a PR is approved (has at least one approval review) +fn is_pr_approved(pr: &PullRequest) -> bool { + use crate::api::PullRequestReviewState; + matches!( + pr.review_state(), + PullRequestReviewState::APPROVED | PullRequestReviewState::MERGED + ) +} + +/// Analyze the stack and create a landing plan +pub fn create_land_plan( + stack: &FlatDep, + repository: &str, + options: &LandOptions, +) -> Result { + if stack.is_empty() { + return Err(LandError::NoPRsInStack); + } + + // Order from base to top + let ordered = order_stack_base_to_top(stack); + + // Filter to only open PRs + let open_prs: Vec<_> = ordered + .into_iter() + .filter(|pr| !pr.is_merged() && pr.state() == &crate::api::PullRequestStatus::Open) + .collect(); + + if open_prs.is_empty() { + return Err(LandError::NoPRsMergeable { + reason: "All PRs are already merged or closed".to_string(), + }); + } + + // Find the target branch (base of the first PR) + let target_branch = stack + .iter() + .find(|(_, parent)| parent.is_none()) + .map(|(pr, _)| pr.base().to_string()) + .unwrap_or_else(|| "main".to_string()); + + // Find mergeable PRs (stopping at first draft or unapproved PR) + let mut mergeable: Vec> = Vec::new(); + + for pr in open_prs.iter() { + // Check for draft PRs + if pr.is_draft() { + if mergeable.is_empty() { + return Err(LandError::DraftBlocking { + pr_number: pr.number(), + }); + } + break; // Draft blocks PRs above it + } + + // Check for approval if required + if options.require_approval && !is_pr_approved(pr) { + if mergeable.is_empty() { + return Err(LandError::ApprovalRequired { + pr_number: pr.number(), + }); + } + break; // Unapproved PR blocks PRs above it + } + + mergeable.push(pr.clone()); + + // Respect max_count + if let Some(max) = options.max_count { + if mergeable.len() >= max { + break; + } + } + } + + if mergeable.is_empty() { + return Err(LandError::NoPRsMergeable { + reason: "No PRs passed approval/draft checks".to_string(), + }); + } + + // Top PR = last in mergeable list (will be merged) + // Rest = PRs to close + let top_pr = mergeable.pop().unwrap(); + let prs_to_close = mergeable; + + Ok(LandPlan { + top_pr, + prs_to_close, + target_branch, + repository: repository.to_string(), + }) +} + +/// Format the dry-run output for a land plan +pub fn format_dry_run(plan: &LandPlan, remaining_prs: &[Rc]) -> String { + let mut output = String::new(); + + output.push_str("Landing Plan:\n"); + output.push_str(&format!(" Target branch: {}\n\n", plan.target_branch)); + + // PRs to land + let total_to_land = plan.prs_to_close.len() + 1; + output.push_str(&format!(" PRs to land ({}):\n", total_to_land)); + + for pr in &plan.prs_to_close { + output.push_str(&format!( + " [x] #{}: {} (will close)\n", + pr.number(), + pr.title() + )); + } + output.push_str(&format!( + " [x] #{}: {} <- will merge\n", + plan.top_pr.number(), + plan.top_pr.title() + )); + + // PRs not included + if !remaining_prs.is_empty() { + output.push_str(&format!( + "\n PRs not included ({}):\n", + remaining_prs.len() + )); + for pr in remaining_prs { + let reason = if pr.is_draft() { + "draft" + } else { + "not approved" + }; + output.push_str(&format!( + " [ ] #{}: {} ({})\n", + pr.number(), + pr.title(), + reason + )); + } + } + + output.push_str("\n Actions that would be taken:\n"); + output.push_str(&format!( + " 1. Update PR #{} base branch: {} -> {}\n", + plan.top_pr.number(), + plan.top_pr.base(), + plan.target_branch + )); + output.push_str(&format!( + " 2. Squash-merge PR #{} into {}\n", + plan.top_pr.number(), + plan.target_branch + )); + + for (i, pr) in plan.prs_to_close.iter().enumerate() { + output.push_str(&format!( + " {}. Close PR #{} with comment: \"Landed via #{}\"\n", + i + 3, + pr.number(), + plan.top_pr.number() + )); + } + + output.push_str("\nRun without --dry-run to execute.\n"); + + output +} + +/// Execute the landing plan +pub async fn execute_land( + plan: &LandPlan, + credentials: &Credentials, +) -> Result { + use crate::api::land::{close_pr_with_comment, merge_pr, update_pr_base}; + + // Step 1: Update top PR's base to target branch + println!( + " Updating PR #{} base to {}...", + plan.top_pr.number(), + plan.target_branch + ); + update_pr_base( + plan.top_pr.number(), + &plan.target_branch, + &plan.repository, + credentials, + ) + .await + .map_err(|e| LandError::ApiError { + message: format!("Failed to update PR base: {}", e), + })?; + + // Step 2: Merge the top PR + println!(" Merging PR #{}...", plan.top_pr.number()); + let merge_url = merge_pr(plan.top_pr.number(), &plan.repository, credentials) + .await + .map_err(|e| LandError::ApiError { + message: format!("Failed to merge PR: {}", e), + })?; + + // Step 3: Close all PRs below with comment + let comment = format!("Landed via #{}", plan.top_pr.number()); + let mut closed_prs = Vec::new(); + + for pr in &plan.prs_to_close { + println!( + " Closing PR #{} (landed via #{})...", + pr.number(), + plan.top_pr.number() + ); + close_pr_with_comment(pr.number(), &comment, &plan.repository, credentials) + .await + .map_err(|e| LandError::ApiError { + message: format!("Failed to close PR #{}: {}", pr.number(), e), + })?; + closed_prs.push(pr.clone()); + } + + Ok(LandResult { + merged_pr: plan.top_pr.clone(), + closed_prs, + merge_url, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::api::{PullRequest, PullRequestStatus}; + + fn make_pr( + number: usize, + head: &str, + base: &str, + approved: bool, + draft: bool, + ) -> Rc { + let reviews = if approved { + vec![crate::api::PullRequestReview::new_for_test( + crate::api::PullRequestReviewState::APPROVED, + )] + } else { + vec![] + }; + + Rc::new(PullRequest::new_for_test( + number, + head, + base, + &format!("PR #{}", number), + PullRequestStatus::Open, + draft, + None, + reviews, + )) + } + + fn make_stack(prs: Vec>) -> FlatDep { + let mut stack = Vec::new(); + for (i, pr) in prs.iter().enumerate() { + let parent = if i > 0 { + Some(prs[i - 1].clone()) + } else { + None + }; + stack.push((pr.clone(), parent)); + } + stack + } + + #[test] + fn test_create_plan_empty_stack() { + let stack: FlatDep = vec![]; + let options = LandOptions::default(); + let result = create_land_plan(&stack, "owner/repo", &options); + assert!(matches!(result, Err(LandError::NoPRsInStack))); + } + + #[test] + fn test_create_plan_single_approved_pr() { + let pr = make_pr(1, "feature-1", "main", true, false); + let stack = make_stack(vec![pr.clone()]); + let options = LandOptions::default(); + + let plan = create_land_plan(&stack, "owner/repo", &options).unwrap(); + + assert_eq!(plan.top_pr.number(), 1); + assert!(plan.prs_to_close.is_empty()); + assert_eq!(plan.target_branch, "main"); + } + + #[test] + fn test_create_plan_all_approved() { + let prs = vec![ + make_pr(1, "feature-1", "main", true, false), + make_pr(2, "feature-2", "feature-1", true, false), + make_pr(3, "feature-3", "feature-2", true, false), + ]; + let stack = make_stack(prs); + let options = LandOptions::default(); + + let plan = create_land_plan(&stack, "owner/repo", &options).unwrap(); + + assert_eq!(plan.top_pr.number(), 3); + assert_eq!(plan.prs_to_close.len(), 2); + assert_eq!(plan.prs_to_close[0].number(), 1); + assert_eq!(plan.prs_to_close[1].number(), 2); + } + + #[test] + fn test_create_plan_partial_approval() { + let prs = vec![ + make_pr(1, "feature-1", "main", true, false), + make_pr(2, "feature-2", "feature-1", true, false), + make_pr(3, "feature-3", "feature-2", false, false), // Not approved + ]; + let stack = make_stack(prs); + let options = LandOptions::default(); + + let plan = create_land_plan(&stack, "owner/repo", &options).unwrap(); + + // Should only include the first two approved PRs + assert_eq!(plan.top_pr.number(), 2); + assert_eq!(plan.prs_to_close.len(), 1); + assert_eq!(plan.prs_to_close[0].number(), 1); + } + + #[test] + fn test_create_plan_first_pr_not_approved() { + let prs = vec![ + make_pr(1, "feature-1", "main", false, false), // Not approved + make_pr(2, "feature-2", "feature-1", true, false), + ]; + let stack = make_stack(prs); + let options = LandOptions::default(); + + let result = create_land_plan(&stack, "owner/repo", &options); + assert!(matches!( + result, + Err(LandError::ApprovalRequired { pr_number: 1 }) + )); + } + + #[test] + fn test_create_plan_draft_blocking() { + let prs = vec![ + make_pr(1, "feature-1", "main", true, true), // Draft + make_pr(2, "feature-2", "feature-1", true, false), + ]; + let stack = make_stack(prs); + let options = LandOptions::default(); + + let result = create_land_plan(&stack, "owner/repo", &options); + assert!(matches!( + result, + Err(LandError::DraftBlocking { pr_number: 1 }) + )); + } + + #[test] + fn test_create_plan_with_count() { + let prs = vec![ + make_pr(1, "feature-1", "main", true, false), + make_pr(2, "feature-2", "feature-1", true, false), + make_pr(3, "feature-3", "feature-2", true, false), + ]; + let stack = make_stack(prs); + let options = LandOptions { + require_approval: true, + max_count: Some(2), + }; + + let plan = create_land_plan(&stack, "owner/repo", &options).unwrap(); + + // Should only include first 2 PRs + assert_eq!(plan.top_pr.number(), 2); + assert_eq!(plan.prs_to_close.len(), 1); + } + + #[test] + fn test_create_plan_no_approval_flag() { + let prs = vec![ + make_pr(1, "feature-1", "main", false, false), // Not approved + make_pr(2, "feature-2", "feature-1", false, false), // Not approved + ]; + let stack = make_stack(prs); + let options = LandOptions { + require_approval: false, + max_count: None, + }; + + let plan = create_land_plan(&stack, "owner/repo", &options).unwrap(); + + // Should include all PRs since approval not required + assert_eq!(plan.top_pr.number(), 2); + assert_eq!(plan.prs_to_close.len(), 1); + } + + #[test] + fn test_order_stack_base_to_top() { + // Create PRs in reverse order + let pr3 = make_pr(3, "feature-3", "feature-2", true, false); + let pr1 = make_pr(1, "feature-1", "main", true, false); + let pr2 = make_pr(2, "feature-2", "feature-1", true, false); + + let stack: FlatDep = vec![ + (pr3.clone(), Some(pr2.clone())), + (pr1.clone(), None), + (pr2.clone(), Some(pr1.clone())), + ]; + + let ordered = order_stack_base_to_top(&stack); + + assert_eq!(ordered[0].number(), 1); + assert_eq!(ordered[1].number(), 2); + assert_eq!(ordered[2].number(), 3); + } +} diff --git a/src/lib.rs b/src/lib.rs index 532aeb0..c3072a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,7 @@ pub mod api; pub mod git; pub mod graph; +pub mod land; pub mod markdown; pub mod persist; pub mod tree; @@ -8,7 +9,7 @@ pub mod util; pub struct Credentials { // Personal access token - token: String, + pub(crate) token: String, } impl Credentials { diff --git a/src/main.rs b/src/main.rs index 4ae6787..f0f9686 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ use std::rc::Rc; use gh_stack::api::PullRequest; use gh_stack::graph::FlatDep; +use gh_stack::land::{self, LandError, LandOptions}; use gh_stack::util::loop_until_confirm; use gh_stack::Credentials; use gh_stack::{api, git, graph, markdown, persist, tree}; @@ -122,6 +123,32 @@ fn clap<'a, 'b>() -> App<'a, 'b> { .arg(exclude.clone()) .arg(identifier.clone()); + let land = SubCommand::with_name("land") + .about("Land a stack of PRs by merging the topmost mergeable PR and closing the rest") + .setting(AppSettings::ArgRequiredElseHelp) + .arg(identifier.clone()) + .arg(exclude.clone()) + .arg(repository.clone()) + .arg( + Arg::with_name("no-approval") + .long("no-approval") + .takes_value(false) + .help("Skip approval requirement check"), + ) + .arg( + Arg::with_name("count") + .long("count") + .takes_value(true) + .value_name("N") + .help("Only land the bottom N PRs in the stack"), + ) + .arg( + Arg::with_name("dry-run") + .long("dry-run") + .takes_value(false) + .help("Preview what would happen without making changes"), + ); + let app = App::new("gh-stack") .setting(AppSettings::SubcommandRequiredElseHelp) .setting(AppSettings::DisableVersion) @@ -130,7 +157,8 @@ fn clap<'a, 'b>() -> App<'a, 'b> { .subcommand(annotate) .subcommand(log) .subcommand(rebase) - .subcommand(autorebase); + .subcommand(autorebase) + .subcommand(land); app } @@ -371,6 +399,116 @@ async fn main() -> Result<(), Box> { println!("All done!"); } + ("land", Some(m)) => { + let identifier = m.value_of("identifier").unwrap(); + + // Get repository from args or environment + let repository = m.value_of("repository").unwrap_or(&repository); + if repository.is_empty() { + panic!( + "You must pass a repository with the -r flag or set GHSTACK_TARGET_REPOSITORY" + ); + } + + println!( + "Analyzing stack for {} in {}...\n", + style(identifier).bold(), + style(repository).bold() + ); + + let stack = + build_pr_stack_for_repo(identifier, repository, &credentials, get_excluded(m)) + .await?; + + if stack.is_empty() { + println!("No PRs found matching '{}'", identifier); + return Ok(()); + } + + // Parse options + let require_approval = !m.is_present("no-approval"); + let max_count = m + .value_of("count") + .map(|s| s.parse::().expect("--count must be a number")); + let dry_run = m.is_present("dry-run"); + + let options = LandOptions { + require_approval, + max_count, + }; + + // Create the landing plan + let plan = match land::create_land_plan(&stack, repository, &options) { + Ok(plan) => plan, + Err(e) => { + match &e { + LandError::ApprovalRequired { pr_number } => { + eprintln!( + "{} PR #{} requires approval", + style("Error:").red().bold(), + pr_number + ); + eprintln!( + " Hint: Get approval for #{}, or use {} to skip this check", + pr_number, + style("--no-approval").cyan() + ); + } + LandError::DraftBlocking { pr_number } => { + eprintln!( + "{} PR #{} is a draft and blocks landing", + style("Error:").red().bold(), + pr_number + ); + eprintln!( + " Hint: Mark PR #{} as ready for review before landing", + pr_number + ); + } + _ => { + eprintln!("{} {}", style("Error:").red().bold(), e); + } + } + std::process::exit(1); + } + }; + + // Calculate remaining PRs (those not in the plan) + let plan_pr_numbers: Vec = std::iter::once(plan.top_pr.number()) + .chain(plan.prs_to_close.iter().map(|pr| pr.number())) + .collect(); + let remaining_prs: Vec> = stack + .iter() + .filter(|(pr, _)| !plan_pr_numbers.contains(&pr.number())) + .filter(|(pr, _)| !pr.is_merged() && pr.state() == &api::PullRequestStatus::Open) + .map(|(pr, _)| pr.clone()) + .collect(); + + if dry_run { + // Print dry-run output + println!("{}", land::format_dry_run(&plan, &remaining_prs)); + return Ok(()); + } + + // Execute the landing + let total_to_land = plan.prs_to_close.len() + 1; + println!("Landing {} PR(s)...\n", total_to_land); + + match land::execute_land(&plan, &credentials).await { + Ok(result) => { + println!( + "\n{} Stack landed via {}", + style("Done!").green().bold(), + style(&result.merge_url).cyan() + ); + } + Err(e) => { + eprintln!("\n{} {}", style("Error:").red().bold(), e); + std::process::exit(1); + } + } + } + (_, _) => panic!("Invalid subcommand."), } From 9c25e3fb95935719e486936ce32b7415aeefbdcb Mon Sep 17 00:00:00 2001 From: Luis Ball Date: Mon, 29 Dec 2025 08:11:46 -0800 Subject: [PATCH 2/4] feat: auto-detect repository from git remote Add automatic repository detection from git remote URL, eliminating the need to pass -r flag or set GHSTACK_TARGET_REPOSITORY when running from inside a git repository. Changes: - Add parse_github_remote_url() to parse SSH/HTTPS URLs (including GitHub Enterprise) - Add detect_repo_from_remote() to extract owner/repo from git remote - Add resolve_repository() helper with fallback chain: -r flag -> env var -> auto-detect - Add --origin/-o flag to specify which remote to use (defaults to 'origin') - Update annotate, log, autorebase, and land commands to use auto-detection - Add unit tests for URL parsing --- src/main.rs | 126 ++++++++++++++++++++++++++++++++-------------------- src/tree.rs | 106 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 47 deletions(-) diff --git a/src/main.rs b/src/main.rs index f0f9686..1ff09e5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -47,12 +47,20 @@ fn clap<'a, 'b>() -> App<'a, 'b> { .takes_value(false) .help("Use shields.io badges for PR status (requires public repo visibility)"); + let origin = Arg::with_name("origin") + .long("origin") + .short("o") + .takes_value(true) + .default_value("origin") + .help("Name of the git remote to detect repository from (default: origin)"); + let annotate = SubCommand::with_name("annotate") .about("Annotate the descriptions of all PRs in a stack with metadata about all PRs in the stack") .setting(AppSettings::ArgRequiredElseHelp) .arg(identifier.clone()) .arg(exclude.clone()) .arg(repository.clone()) + .arg(origin.clone()) .arg(ci.clone()) .arg(prefix.clone()) .arg(badges.clone()) @@ -74,6 +82,7 @@ fn clap<'a, 'b>() -> App<'a, 'b> { .arg(identifier.clone().index(2)) .arg(exclude.clone()) .arg(repository.clone()) + .arg(origin.clone()) .arg( Arg::with_name("project") .long("project") @@ -129,6 +138,7 @@ fn clap<'a, 'b>() -> App<'a, 'b> { .arg(identifier.clone()) .arg(exclude.clone()) .arg(repository.clone()) + .arg(origin.clone()) .arg( Arg::with_name("no-approval") .long("no-approval") @@ -209,6 +219,45 @@ fn get_excluded(m: &ArgMatches) -> Vec { } } +/// Resolve the repository to use, with fallback chain: +/// 1. -r flag (explicit override) +/// 2. GHSTACK_TARGET_REPOSITORY env var +/// 3. Auto-detect from git remote +fn resolve_repository( + arg_value: Option<&str>, + env_value: &str, + remote_name: &str, +) -> Result { + // Priority 1: Explicit -r flag + if let Some(repo) = arg_value { + if !repo.is_empty() { + return Ok(repo.to_string()); + } + } + + // Priority 2: Environment variable + if !env_value.is_empty() { + return Ok(env_value.to_string()); + } + + // Priority 3: Auto-detect from git remote + if let Some(repo) = tree::detect_repo_from_remote(remote_name) { + eprintln!( + "Detected repository: {} (from {} remote)", + style(&repo).cyan(), + remote_name + ); + return Ok(repo); + } + + Err(format!( + "Could not determine repository. Either:\n \ + - Run from inside a git repo with a GitHub remote\n \ + - Set GHSTACK_TARGET_REPOSITORY environment variable\n \ + - Use the -r flag" + )) +} + fn remove_title_prefixes(title: String, prefix: &str) -> String { let regex = Regex::new(&format!("[{}]", prefix).to_string()).unwrap(); regex.replace_all(&title, "").into_owned() @@ -231,26 +280,21 @@ async fn main() -> Result<(), Box> { let prefix = regex::escape(prefix); // if ci flag is set, set ci to true let ci = m.is_present("ci"); - // replace it with the -r argument value if set - let repository = m.value_of("repository").unwrap_or(&repository); - // if repository is still unset, throw an error - if repository.is_empty() { - let error = format!( - "Invalid target repository {repo}. You must pass a repository with the -r flag or set GHSTACK_TARGET_REPOSITORY", repo = repository - ); - panic!("{}", error); - } + // resolve repository with fallback chain + let remote_name = m.value_of("origin").unwrap_or("origin"); + let repository = resolve_repository(m.value_of("repository"), &repository, remote_name) + .unwrap_or_else(|e| panic!("{}", e)); let identifier = remove_title_prefixes(identifier.to_string(), &prefix); println!( "Searching for {} identifier in {} repo", style(&identifier).bold(), - style(repository).bold() + style(&repository).bold() ); let stack = - build_pr_stack_for_repo(&identifier, repository, &credentials, get_excluded(m)) + build_pr_stack_for_repo(&identifier, &repository, &credentials, get_excluded(m)) .await?; let use_badges = m.is_present("badges"); @@ -258,7 +302,7 @@ async fn main() -> Result<(), Box> { &stack, &identifier, m.value_of("prelude"), - repository, + &repository, use_badges, ); @@ -288,22 +332,18 @@ async fn main() -> Result<(), Box> { } }; - // replace it with the -r argument value if set - let repository = m.value_of("repository").unwrap_or(&repository); - // if repository is still unset, throw an error - if repository.is_empty() { - panic!( - "You must pass a repository with the -r flag or set GHSTACK_TARGET_REPOSITORY" - ); - } + // resolve repository with fallback chain + let remote_name = m.value_of("origin").unwrap_or("origin"); + let repository = resolve_repository(m.value_of("repository"), &repository, remote_name) + .unwrap_or_else(|e| panic!("{}", e)); println!( "Searching for {} identifier in {} repo", style(identifier).bold(), - style(repository).bold() + style(&repository).bold() ); let stack = - build_pr_stack_for_repo(identifier, repository, &credentials, get_excluded(m)) + build_pr_stack_for_repo(identifier, &repository, &credentials, get_excluded(m)) .await?; // Check for empty stack @@ -356,24 +396,20 @@ async fn main() -> Result<(), Box> { ("autorebase", Some(m)) => { let identifier = m.value_of("identifier").unwrap(); - // store the value of GHSTACK_TARGET_REPOSITORY - let repository = env::var("GHSTACK_TARGET_REPOSITORY").unwrap_or_default(); - // replace it with the -r argument value if set - let repository = m.value_of("repository").unwrap_or(&repository); - // if repository is still unset, throw an error - if repository.is_empty() { - panic!( - "You must pass a repository with the -r flag or set GHSTACK_TARGET_REPOSITORY" - ); - } + // defaults to "origin" if no remote is specified + let remote_name = m.value_of("origin").unwrap_or("origin"); + + // resolve repository with fallback chain + let repository = resolve_repository(m.value_of("repository"), &repository, remote_name) + .unwrap_or_else(|e| panic!("{}", e)); println!( "Searching for {} identifier in {} repo", style(identifier).bold(), - style(repository).bold() + style(&repository).bold() ); let stack = - build_pr_stack_for_repo(identifier, repository, &credentials, get_excluded(m)) + build_pr_stack_for_repo(identifier, &repository, &credentials, get_excluded(m)) .await?; let project = m @@ -381,9 +417,8 @@ async fn main() -> Result<(), Box> { .expect("The --project argument is required."); let project = Repository::open(project)?; - // defaults to "origin" if no remote is specified - let remote = m.value_of("origin").unwrap_or("origin"); - let remote = project.find_remote(remote).unwrap(); + // use the same remote name for finding the remote to push to + let remote = project.find_remote(remote_name).unwrap(); // if ci flag is set, set ci to true let ci = m.is_present("ci"); @@ -402,22 +437,19 @@ async fn main() -> Result<(), Box> { ("land", Some(m)) => { let identifier = m.value_of("identifier").unwrap(); - // Get repository from args or environment - let repository = m.value_of("repository").unwrap_or(&repository); - if repository.is_empty() { - panic!( - "You must pass a repository with the -r flag or set GHSTACK_TARGET_REPOSITORY" - ); - } + // resolve repository with fallback chain + let remote_name = m.value_of("origin").unwrap_or("origin"); + let repository = resolve_repository(m.value_of("repository"), &repository, remote_name) + .unwrap_or_else(|e| panic!("{}", e)); println!( "Analyzing stack for {} in {}...\n", style(identifier).bold(), - style(repository).bold() + style(&repository).bold() ); let stack = - build_pr_stack_for_repo(identifier, repository, &credentials, get_excluded(m)) + build_pr_stack_for_repo(identifier, &repository, &credentials, get_excluded(m)) .await?; if stack.is_empty() { @@ -438,7 +470,7 @@ async fn main() -> Result<(), Box> { }; // Create the landing plan - let plan = match land::create_land_plan(&stack, repository, &options) { + let plan = match land::create_land_plan(&stack, &repository, &options) { Ok(plan) => plan, Err(e) => { match &e { diff --git a/src/tree.rs b/src/tree.rs index 0e026f6..fceded7 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -66,6 +66,45 @@ pub fn detect_repo() -> Option { Repository::discover(".").ok() } +/// Try to detect repository (owner/repo) from git remote +/// +/// # Arguments +/// * `remote_name` - Name of the remote to use (typically "origin") +pub fn detect_repo_from_remote(remote_name: &str) -> Option { + let repo = detect_repo()?; + let remote = repo.find_remote(remote_name).ok()?; + let url = remote.url()?; + parse_github_remote_url(url) +} + +/// Parse a GitHub remote URL to extract owner/repo +/// +/// Handles: +/// - SSH: git@github.com:owner/repo.git +/// - SSH (Enterprise): git@github.mycompany.com:owner/repo.git +/// - HTTPS: https://github.com/owner/repo.git +/// - HTTPS (Enterprise): https://github.mycompany.com/owner/repo.git +/// - Without .git suffix +fn parse_github_remote_url(url: &str) -> Option { + // SSH format: git@:owner/repo.git + if url.starts_with("git@") { + let path = url.split(':').nth(1)?; + let repo = path.trim_end_matches(".git"); + return Some(repo.to_string()); + } + + // HTTPS format: https:///owner/repo.git + if url.starts_with("https://") || url.starts_with("http://") { + let without_protocol = url.split("://").nth(1)?; + // Skip the host part, get everything after first / + let path = without_protocol.splitn(2, '/').nth(1)?; + let repo = path.trim_end_matches(".git"); + return Some(repo.to_string()); + } + + None +} + /// Get current branch name from repo pub fn current_branch(repo: &Repository) -> Option { repo.head().ok()?.shorthand().map(String::from) @@ -1162,4 +1201,71 @@ mod tests { let output = render(&entries, &config, true); insta::assert_snapshot!(output); } + + // Tests for parse_github_remote_url + #[test] + fn test_parse_github_remote_url_ssh() { + assert_eq!( + parse_github_remote_url("git@github.com:owner/repo.git"), + Some("owner/repo".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_ssh_no_suffix() { + assert_eq!( + parse_github_remote_url("git@github.com:owner/repo"), + Some("owner/repo".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_https() { + assert_eq!( + parse_github_remote_url("https://github.com/owner/repo.git"), + Some("owner/repo".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_https_no_suffix() { + assert_eq!( + parse_github_remote_url("https://github.com/owner/repo"), + Some("owner/repo".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_http() { + assert_eq!( + parse_github_remote_url("http://github.com/owner/repo.git"), + Some("owner/repo".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_enterprise_ssh() { + assert_eq!( + parse_github_remote_url("git@github.mycompany.com:org/project.git"), + Some("org/project".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_enterprise_https() { + assert_eq!( + parse_github_remote_url("https://github.mycompany.com/org/project.git"), + Some("org/project".to_string()) + ); + } + + #[test] + fn test_parse_github_remote_url_invalid() { + assert_eq!(parse_github_remote_url("not-a-url"), None); + } + + #[test] + fn test_parse_github_remote_url_empty() { + assert_eq!(parse_github_remote_url(""), None); + } } From bae2a046d9090fe509b1562b5ada67fa0bc69a93 Mon Sep 17 00:00:00 2001 From: Luis Ball Date: Mon, 29 Dec 2025 08:17:51 -0800 Subject: [PATCH 3/4] docs: update README for land command and auto-detect feature --- README.md | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 053f14d..ca53901 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ With this graph built up, the tool can: - Add a markdown table to the PR description (idempotently) of each PR in the stack describing _all_ PRs in the stack. - Log a simple list of all PRs in the stack (+ dependencies) to stdout. - Automatically update the stack + push after making local changes. +- **Land an entire stack** by squash-merging the topmost approved PR and closing the rest. --- @@ -66,7 +67,8 @@ cargo install --force --path . # Set the environment variable for the Github API token $ export GHSTACK_OAUTH_TOKEN='' -# Set the environment variable for the repository owner/name +# Optional: The repository is auto-detected from your git remote. +# You can override it with an environment variable or the -r flag: $ export GHSTACK_TARGET_REPOSITORY='' # You can also set these in a `.gh-stack.env` file in the project root. @@ -84,17 +86,18 @@ FLAGS: SUBCOMMANDS: annotate Annotate the descriptions of all PRs in a stack with metadata about all PRs in the stack autorebase Rebuild a stack based on changes to local branches and mirror these changes up to the remote + land Land a stack by squash-merging the topmost approved PR and closing the rest log Print a list of all pull requests in a stack to STDOUT rebase Print a bash script to STDOUT that can rebase/update the stack (with a little help) -# Print a description of the stack to stdout. for a specific repository. +# Print a description of the stack to stdout (auto-detects repository from git remote) $ gh-stack log 'stack-identifier' -# # Idempotently add a markdown table summarizing the stack -# to the description of each PR in the stack for a specific repository. +# Idempotently add a markdown table summarizing the stack +# to the description of each PR in the stack $ gh-stack annotate 'stack-identifier' -# Same as above, but for the specified repository. +# Override auto-detected repository with -r flag $ gh-stack annotate 'stack-identifier' -r '' # Same as above, but with a custom title prefix. @@ -116,6 +119,18 @@ $ gh-stack autorebase 'stack-identifier' -C /path/to/repo # Same as above, but skips confirmation step. $ gh-stack autorebase 'stack-identifier' -C /path/to/repo --ci +# Land the entire stack (squash-merges topmost approved PR, closes the rest) +$ gh-stack land 'stack-identifier' + +# Preview what would happen without making changes +$ gh-stack land 'stack-identifier' --dry-run + +# Skip approval requirement check +$ gh-stack land 'stack-identifier' --no-approval + +# Only land the bottom N PRs in the stack +$ gh-stack land 'stack-identifier' --count 2 + # Emit a bash script that can update a stack in the case of conflicts. # WARNING: This script could potentially cause destructive behavior. $ gh-stack rebase 'stack-identifier' From fe029c8c9e862679a77ee82ffafdf3cb409ecaa7 Mon Sep 17 00:00:00 2001 From: Luis Ball Date: Mon, 29 Dec 2025 08:20:22 -0800 Subject: [PATCH 4/4] fix: resolve clippy warnings --- src/main.rs | 5 ++--- src/tree.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index 1ff09e5..e040a9e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -250,12 +250,11 @@ fn resolve_repository( return Ok(repo); } - Err(format!( - "Could not determine repository. Either:\n \ + Err("Could not determine repository. Either:\n \ - Run from inside a git repo with a GitHub remote\n \ - Set GHSTACK_TARGET_REPOSITORY environment variable\n \ - Use the -r flag" - )) + .to_string()) } fn remove_title_prefixes(title: String, prefix: &str) -> String { diff --git a/src/tree.rs b/src/tree.rs index fceded7..b352e1b 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -97,7 +97,7 @@ fn parse_github_remote_url(url: &str) -> Option { if url.starts_with("https://") || url.starts_with("http://") { let without_protocol = url.split("://").nth(1)?; // Skip the host part, get everything after first / - let path = without_protocol.splitn(2, '/').nth(1)?; + let path = without_protocol.split_once('/')?.1; let repo = path.trim_end_matches(".git"); return Some(repo.to_string()); }