Skip to content

remove name and merge commands #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
slinder1 merged 1 commit into from
Aug 11, 2026
Merged

remove name and merge commands #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 0 additions & 56 deletions src/change.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,53 +143,6 @@ pub struct Change {
}

impl Change {
pub fn render_pr_ui(
&self,
changes: &[Self],
short_name: &str,
merged_prs: &[Pr],
) -> Result<()> {
let commit = self.local_change.commit()?;
let mut index = None;
let title = String::from(
commit
.summary()
.context("failed to get commit summary")?
.context("commit has no summary")?,
);
let mut body = String::from(
commit
.body()
.context("failed to get commit body")?
.context("commit has no body")?,
);
body.push_str("\n\n---\n\n");
body.push_str("**Stack**:\n");
for (i, c) in changes.iter().enumerate() {
body.push_str(&format!("- #{}", c.pr.number));
if self.pr.number == c.pr.number {
index = Some(i);
body.push('⬅');
}
body.push('\n');
}
let index = index.expect(
"render_pr_ui asked to render into a stack of changes which does not contain self?",
);
for pr in merged_prs.iter().rev() {
body.push_str(&format!("- #{}\n", pr.number));
}
body.push_str(&format!("- `{}`\n\n<sub>(Note: Closed and merged PRs may not be reflected here and PR numbering is not stable.)</sub>\n", env::get().base_branch()));
let count = changes.len() + merged_prs.len();
let position = count - index;
let prefix = if short_name.is_empty() {
"".into()
} else {
format!("{}: ", short_name)
};
self.pr
.set_title_and_body(&format!("[{prefix}{position}/{count}]: {title}"), &body)
}
/// Adapted from https://joshcannon.me/2025/04/05/pr-interdiff.html
pub fn interdiff(&self) -> Result<String> {
let change = self.local_change.id.as_str();
Expand Down Expand Up @@ -236,15 +189,6 @@ impl Change {
.with_context(|| format!("failed to generate interdiff for change {change}"))?;
Ok(out)
}
pub fn merge(&self) -> Result<()> {
let commit = self.local_change.commit()?;
let subject_raw = commit.summary()?.context("couldn't get summary")?;
let subject = format!("{} (#{})", subject_raw, self.pr.number);
let body = commit.body()?.context("couldn't get summary")?;
let sha = format!("{}", self.local_change.oid);
self.pr.merge(&subject, body, &sha)?;
Ok(())
}
}

fn tree<'repo>(commit: &Commit<'repo>) -> Result<Tree<'repo>> {
Expand Down
46 changes: 2 additions & 44 deletions src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ use clap::{ArgAction, Args, Parser, Subcommand};
///
/// * Never touches your local branches. The tool only reads from your local branch and attempts to
/// mirror it to GitHub by: fetching remote tracking branches, force-pushing namespaced refs,
/// creating PRs, and maintaining PR bodies and comments to present a pseudo-UI for the stack. If
/// you use the `merge` subcommand, it also touches namespaced git-config entries under
/// `branch.<name>.praddle-*`, but this is only for optional features.
/// creating PRs and comments with the official GitHub UI.
/// * Treats one branch as one patch-stack, where each commit maps 1:1 to a PR.
/// * Uses the same "Change-Id" trailer used by Gerrit. You can install the commit-msg hook from
/// a Gerrit instance or use the install-hook subcommand to install an embedded copy.
Expand All @@ -34,11 +32,6 @@ use clap::{ArgAction, Args, Parser, Subcommand};
/// but an extremely short-lived review process. Ideas about how to potentially resolve this is
/// documented at https://github.com/slinder1/praddle/blob/main/IDEAS.md and contributions are
/// welcome!
/// * Can lose track of merged/closed PR if the user is not careful to use the `merge` subcommand.
/// This may be mildly confusing, but is more-or-less by design: the change commit which corresponds
/// to a merged PR will naturally disappear from the branch on rebase. The `merge` subcommand
/// notes the Change-Id of successful merges in a git-config entry tied to the branch, so
/// it doesn't forget to count them and link to them, but it is purely aesthetics.
/// * Currently lacks a lot of polish and documentation.
///
/// It reads configuration from the first of the following:
Expand All @@ -54,12 +47,6 @@ use clap::{ArgAction, Args, Parser, Subcommand};
/// base_branch = "main"
/// user_branch_prefix = "users/$USER/"
///
/// The title of PR number `N` in a series of `M` commits is prefixed with:
///
/// * `[<branch-desc-first-line>: N/M]: ` if the branch has a description,
/// editable via `git branch --edit-description`, or
/// * `[N/M]: ` otherwise.
///
#[derive(Parser)]
#[command(version, verbatim_doc_comment, args_override_self = true)]
pub struct Cli {
Expand Down Expand Up @@ -103,33 +90,12 @@ pub enum Command {
/// force-pushed to a corresponding branch named `${user_branch_prefix}${change_id}` on
/// `${remote}`. Each commit will be matched to its existing PR or else a new PR will be
/// created for it. The PRs will be "stacked" such that they reproduce the local branch
/// sequence, with additional trailers in the PR message body to help reviewers navigate the
/// stack.
/// sequence.
///
/// Note: This command will never modify your commits or refs, even their messages. No local
/// branches are created or destroyed. All mutation occurs exclusively on the `$remote`.
#[command(visible_alias = "p")]
Push(Push),
/// With no arguments, print the current stack's short-name. With an argument, set it.
///
/// The short-name is tracked in the `branch.<name>.praddle-shortName` git config entry,
/// and is used to prefix the PR title, e.g. `[${short-name} 3/5] ...`
Name(Name),
/// Merge the next change.
///
/// If successful, this will modify the local `branch.<name>.praddle-mergedChangeIds` git config
/// entry to record that the change was merged. This allows future `praddle push`es to include
/// merged changes in the reviewer stack "UI", keeping the relative numbering of changes stable.
///
/// If a change's PR is merged in any other way, it will "disappear" from the stack, affecting
/// all downstream numbering and the total number of changes in the stack. If you want to
/// manually correct this, edit `branch.<name>.praddle-mergedChangeIds` (a ':' separated list) using
/// e.g. `git config --edit` and append the merged change's ID to the list (or create it, if it
/// does not already exist).
///
/// If you don't care about the renumbering behavior, you can safely ignore this subcommand (it
/// is purely aesthetic).
Merge(Merge),
/// Print the PR URL of the top-most (i.e. last) change which already has one.
Url(Url),
/// Install a commit-msg hook in the current git repo to create `Change-Id:` trailers.
Expand All @@ -152,14 +118,6 @@ pub struct Push {
pub draft: bool,
}

#[derive(Args)]
pub struct Name {
pub new_name: Option<String>,
}

#[derive(Args)]
pub struct Merge;

#[derive(Args)]
pub struct Url;

Expand Down
35 changes: 0 additions & 35 deletions src/gh.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,6 @@ impl Pr {
self.state == state
}

pub fn set_title_and_body(&self, title: &str, body: &str) -> Result<()> {
let mut body_arg = ArgInlineOrFile::new("body");
let mut cmd = gh();
let args = self.args_for("edit", [format!("--title={title}"), body_arg.arg(body)?])?;
cmd.args(args);
exec!(env::get(), dry_return = (), cmd);
Ok(())
}

pub fn mark_ready(&self, ready: bool) -> Result<()> {
let mut cmd = gh();
let opts = if ready {
Expand Down Expand Up @@ -226,32 +217,6 @@ impl Pr {
bail!("gh pr create did not produce a URL")
}

pub fn merge(&self, subject: &str, body: &str, sha: &str) -> Result<()> {
let mut cmd = gh();
let mut body_arg = ArgInlineOrFile::new("body");
let body_arg_string = body_arg.arg(body)?;
let args = self.args_for(
"merge",
[
"--squash",
"--match-head-commit",
sha,
"--subject",
subject,
&body_arg_string,
],
)?;
cmd.args(args);
let output = exec!(env::get(), dry_return = (), cmd);
// gh cli doesn't consider this a failure, but we want to so we don't mistakenly add an
// already-merged change to the metadata. We could instead infer that the change should be
// added to the metadata, but we can't necessarily assume it is the *next* merged change(?)
if String::from_utf8_lossy(output.stderr.as_ref()).contains("was already merged") {
bail!("pr {} was already merged", self.number);
}
Ok(())
}

pub fn get_url(&self) -> Result<String> {
Ok(format!("{}/pull/{}", build_repo_url()?, self.number))
}
Expand Down
1 change: 0 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ mod change;
mod cli;
mod env;
mod gh;
mod metadata;
mod praddle;
mod util;

Expand Down
53 changes: 0 additions & 53 deletions src/metadata.rs
View file
Open in desktop

This file was deleted.

58 changes: 1 addition & 57 deletions src/praddle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::change::{self, AnyChange, Change, LocalChange};
use crate::cli;
use crate::env;
use crate::gh::{self, Pr, PrState};
use crate::metadata::StackMetadata;
use crate::util::Extract;
use anyhow::{Context, Result, bail};
use git2::Repository;
Expand All @@ -31,17 +30,13 @@ pub fn praddle(cli: cli::Cli) -> Result<()> {
}
match cli.command {
cli::Command::Push(ref args) => push(args),
cli::Command::Name(ref args) => name(args),
cli::Command::Merge(ref args) => merge(args),
cli::Command::Url(ref args) => url(args),
cli::Command::InstallHook(_) => unreachable!(),
}
}

fn push(args: &cli::Push) -> Result<()> {
let env = env::get();
let stack_meta =
StackMetadata::from_repo().context("could not parse branch description metadata")?;
let mut reviewers = vec![];
for group_key in args.reviewer_groups.iter() {
let group = env
Expand All @@ -55,20 +50,6 @@ fn push(args: &cli::Push) -> Result<()> {
change::get_local_changes().context("could not enumerate current local branch")?;
let mut prs_by_change_id = gh::prs_by_change_id(|pr| !pr.in_state(PrState::Closed))
.context("could not enumerate remote prs")?;
let mut merged_prs = vec![];
for merged_change_id in stack_meta.merged_change_ids {
let pr = prs_by_change_id
.remove(&merged_change_id)
.with_context(|| format!("merged change {} has no pr", merged_change_id))?;
if pr.in_state(PrState::Open) {
bail!(
"pr {} for merged change {} is still open",
pr.number,
merged_change_id,
);
}
merged_prs.push(pr);
}
let mut any_changes = vec![];
for local_change in local_changes {
any_changes.push(match prs_by_change_id.remove(&local_change.id) {
Expand Down Expand Up @@ -159,8 +140,7 @@ fn push(args: &cli::Push) -> Result<()> {
c.pr.number, base,
)
})?;
c.render_pr_ui(&changes, &stack_meta.short_name, &merged_prs)
.context("could not render pseudo-ui in pr title/body")
Ok(())
})
.collect::<Result<Vec<_>>>()
.context("could not set pr bases and bodies")?;
Expand Down Expand Up @@ -200,42 +180,6 @@ fn detect_cycles(any_changes: &[AnyChange]) -> bool {
false
}

fn name(args: &cli::Name) -> Result<()> {
let mut stack_meta = StackMetadata::from_repo()?;
match args.new_name {
None => println!("{}", stack_meta.short_name),
Some(ref new_name) => {
stack_meta.short_name = new_name.to_string();
stack_meta.to_repo()?;
}
}
Ok(())
}

fn merge(_args: &cli::Merge) -> Result<()> {
let mut stack_meta =
StackMetadata::from_repo().context("could not parse branch description metadata")?;
let local_change = change::get_local_changes()
.context("could not enumerate current local branch")?
.pop()
.context("no local changes")?;
if stack_meta.merged_change_ids.contains(&local_change.id) {
bail!("change {} was already merged", local_change.id);
}
let mut prs_by_change_id = gh::prs_by_change_id(|pr| !pr.in_state(PrState::Closed))
.context("could not enumerate remote prs")?;
let pr = prs_by_change_id
.remove(&local_change.id)
.with_context(|| format!("change {} has no pr", local_change.id))?;
let change = Change { local_change, pr };
change.merge()?;
stack_meta
.merged_change_ids
.push(change.local_change.id.clone());
stack_meta.to_repo()?;
Ok(())
}

fn url(_args: &cli::Url) -> Result<()> {
let local_changes =
change::get_local_changes().context("could not enumerate current local branch")?;
Expand Down
Loading
Loading