Skip to content

Commit

Permalink
Rename the request-review-branch command to branch
Browse files Browse the repository at this point in the history
Rename the request-review-branch command to branch because now that it
does everything branch did and more we don't need the distinction
between the two separate commands. Also it makes it clearer as to what
is actually going on.

[changelog]
changed: the request-review-branch command to just be branch
added: the command alias "b" to the new branch command

<!-- ps-id: dbcfdcc5-e26a-49da-9fdb-f4cf64270a33 -->
  • Loading branch information
drewdeponte committed Oct 20, 2023
1 parent abb9177 commit dbfda73
Show file tree
Hide file tree
Showing 12 changed files with 80 additions and 95 deletions.
10 changes: 4 additions & 6 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub struct BatchRequestReview {
}

#[derive(Debug, Args)]
pub struct RequestReviewBranchCmdOpts {
pub struct BranchCmdOpts {
pub patch_index_or_range: String,
/// Use the provided branch name instead of generating one
#[arg(short = 'n')]
Expand Down Expand Up @@ -73,11 +73,9 @@ pub struct BackupStackCmdOpts {

#[derive(Debug, Subcommand)]
pub enum Command {
/// Create a request review branch on the patch stack base, cherry-pick
/// the specified patch onto it, & record association between patch &
/// branch
#[command(name = "request-review-branch")]
RequestReviewBranch(RequestReviewBranchCmdOpts),
/// (b) - Create a branch for a patch or patch series
#[command(name = "branch", alias = "b")]
Branch(BranchCmdOpts),
/// (int) - Integrate the specified patch into the patch stacks upstream
/// remote
#[command(name = "integrate", alias = "int")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use std::option::Option;
use std::str::FromStr;
use std::string::String;

pub fn request_review_branch(patch_index_or_range: String, branch_name: Option<String>) {
pub fn branch(patch_index_or_range: String, branch_name: Option<String>) {
match PatchIndexRange::from_str(&patch_index_or_range) {
Ok(patch_index_range) => {
let res = ps::request_review_branch(
let res = ps::branch(
patch_index_range.start_index,
patch_index_range.end_index,
branch_name,
Expand Down
2 changes: 1 addition & 1 deletion src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

pub mod backup_stack;
pub mod batch_request_review;
pub mod branch;
pub mod checkout;
pub mod fetch;
pub mod integrate;
Expand All @@ -14,7 +15,6 @@ pub mod patch_index_range;
pub mod pull;
pub mod rebase;
pub mod request_review;
pub mod request_review_branch;
pub mod show;
pub mod sync;
pub mod utils;
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod ps;
pub use ps::private::config::{get_config, GetConfigError, PsConfig};
pub use ps::public::backup_stack::backup_stack;
pub use ps::public::batch_request_review::{batch_request_review, BatchRequestReviewError};
pub use ps::public::branch::{branch, BranchError};
pub use ps::public::checkout::checkout;
pub use ps::public::fetch::fetch;
pub use ps::public::integrate;
Expand All @@ -15,7 +16,6 @@ pub use ps::public::list::list;
pub use ps::public::pull::{pull, PullError};
pub use ps::public::rebase::rebase;
pub use ps::public::request_review::{request_review, RequestReviewError};
pub use ps::public::request_review_branch::{request_review_branch, RequestReviewBranchError};
pub use ps::public::show::show;
pub use ps::public::sync::{sync, SyncError};
pub use ps::public::upstream_patches::upstream_patches;
Expand Down
7 changes: 2 additions & 5 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,8 @@ fn main() {
let cli = cli::Cli::parse();

match cli.command {
cli::Command::RequestReviewBranch(opts) => {
commands::request_review_branch::request_review_branch(
opts.patch_index_or_range,
opts.branch_name,
)
cli::Command::Branch(opts) => {
commands::branch::branch(opts.patch_index_or_range, opts.branch_name)
}
cli::Command::Integrate(opts) => commands::integrate::integrate(
opts.patch_index_or_range,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::result::Result;
use uuid::Uuid;

#[derive(Debug)]
pub enum RequestReviewBranchError {
pub enum BranchError {
RepositoryMissing,
PatchStackNotFound,
PatchStackBaseNotFound,
Expand All @@ -32,90 +32,90 @@ pub enum RequestReviewBranchError {
PatchIndexRangeOutOfBounds(ps::PatchRangeWithinStackBoundsError),
}

impl From<git::CreateCwdRepositoryError> for RequestReviewBranchError {
impl From<git::CreateCwdRepositoryError> for BranchError {
fn from(_e: git::CreateCwdRepositoryError) -> Self {
RequestReviewBranchError::RepositoryMissing
BranchError::RepositoryMissing
}
}

impl From<ps::PatchStackError> for RequestReviewBranchError {
impl From<ps::PatchStackError> for BranchError {
fn from(e: ps::PatchStackError) -> Self {
match e {
ps::PatchStackError::GitError(_git2_error) => {
RequestReviewBranchError::PatchStackNotFound
BranchError::PatchStackNotFound
}
ps::PatchStackError::HeadNoName => RequestReviewBranchError::PatchStackNotFound,
ps::PatchStackError::HeadNoName => BranchError::PatchStackNotFound,
ps::PatchStackError::UpstreamBranchNameNotFound => {
RequestReviewBranchError::PatchStackNotFound
BranchError::PatchStackNotFound
}
}
}
}

impl fmt::Display for RequestReviewBranchError {
impl fmt::Display for BranchError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
RequestReviewBranchError::RepositoryMissing => {
BranchError::RepositoryMissing => {
write!(f, "Repository not found in current working directory")
}
RequestReviewBranchError::PatchStackNotFound => write!(f, "Patch Stack not found"),
RequestReviewBranchError::PatchStackBaseNotFound => {
BranchError::PatchStackNotFound => write!(f, "Patch Stack not found"),
BranchError::PatchStackBaseNotFound => {
write!(f, "Patch Stack Base not found")
}
RequestReviewBranchError::PatchIndexNotFound => write!(f, "Patch Index out of range"),
RequestReviewBranchError::PatchCommitNotFound => write!(f, "Patch commit not found"),
RequestReviewBranchError::PatchMessageMissing => write!(f, "Patch missing message"),
RequestReviewBranchError::PatchSummaryMissing => write!(f, "Patch missing summary"),
RequestReviewBranchError::CreateRrBranchFailed => {
BranchError::PatchIndexNotFound => write!(f, "Patch Index out of range"),
BranchError::PatchCommitNotFound => write!(f, "Patch commit not found"),
BranchError::PatchMessageMissing => write!(f, "Patch missing message"),
BranchError::PatchSummaryMissing => write!(f, "Patch missing summary"),
BranchError::CreateRrBranchFailed => {
write!(f, "Failed to create request-review branch")
}
RequestReviewBranchError::RrBranchNameNotUtf8 => {
BranchError::RrBranchNameNotUtf8 => {
write!(f, "request-review branch is not utf8")
}
RequestReviewBranchError::CherryPickFailed(_git_error) => {
BranchError::CherryPickFailed(_git_error) => {
write!(f, "Failed to cherry pick")
}
RequestReviewBranchError::GetPatchListFailed(_patch_list_error) => {
BranchError::GetPatchListFailed(_patch_list_error) => {
write!(f, "Failed to get patch list")
}
RequestReviewBranchError::GetPatchMetaDataPathFailed(_patch_meta_data_path_error) => {
BranchError::GetPatchMetaDataPathFailed(_patch_meta_data_path_error) => {
write!(
f,
"Failed to get patch meta data path {:?}",
_patch_meta_data_path_error
)
}
RequestReviewBranchError::OpenGitConfigFailed(_) => {
BranchError::OpenGitConfigFailed(_) => {
write!(f, "Failed to open git config")
}
RequestReviewBranchError::PatchCommitDiffPatchIdFailed(_) => {
BranchError::PatchCommitDiffPatchIdFailed(_) => {
write!(f, "Failed to get commit diff patch id")
}
RequestReviewBranchError::PatchStackHeadNoName => {
BranchError::PatchStackHeadNoName => {
write!(f, "Patch Stack Head has no name")
}
RequestReviewBranchError::GetListPatchInfoFailed(_get_list_patch_info_error) => {
BranchError::GetListPatchInfoFailed(_get_list_patch_info_error) => {
write!(f, "Failed to get list of patch Git info")
}
RequestReviewBranchError::PatchBranchAmbiguous => {
BranchError::PatchBranchAmbiguous => {
write!(
f,
"Patch Branch is Ambiguous - more than one branch associated with patch"
)
}
RequestReviewBranchError::AddPatchIdsFailed(_) => {
BranchError::AddPatchIdsFailed(_) => {
write!(f, "Failed to add patch ids to commits in the patch stack")
}
RequestReviewBranchError::PatchIndexRangeOutOfBounds(_) => {
BranchError::PatchIndexRangeOutOfBounds(_) => {
write!(f, "Patch index range out of patch stack bounds")
}
RequestReviewBranchError::AssociatedBranchAmbiguous(_) => {
BranchError::AssociatedBranchAmbiguous(_) => {
write!(
f,
"The associated branch is ambiguous. Please specify the branch explicitly."
)
}
RequestReviewBranchError::PatchSeriesRequireBranchName => {
BranchError::PatchSeriesRequireBranchName => {
write!(
f,
"When creating a patch series you must specify the branch name."
Expand All @@ -125,39 +125,39 @@ impl fmt::Display for RequestReviewBranchError {
}
}

pub fn request_review_branch(
pub fn branch(
repo: &git2::Repository,
start_patch_index: usize,
end_patch_index: Option<usize>,
given_branch_name_option: Option<String>,
) -> Result<(git2::Branch<'_>, git2::Oid), RequestReviewBranchError> {
) -> Result<(git2::Branch<'_>, git2::Oid), BranchError> {
let config =
git2::Config::open_default().map_err(RequestReviewBranchError::OpenGitConfigFailed)?;
git2::Config::open_default().map_err(BranchError::OpenGitConfigFailed)?;

ps::add_patch_ids(repo, &config).map_err(RequestReviewBranchError::AddPatchIdsFailed)?;
ps::add_patch_ids(repo, &config).map_err(BranchError::AddPatchIdsFailed)?;

let patch_stack = ps::get_patch_stack(repo)?;
let patches_vec = ps::get_patch_list(repo, &patch_stack)
.map_err(RequestReviewBranchError::GetPatchListFailed)?;
.map_err(BranchError::GetPatchListFailed)?;

// validate patch indexes are within bounds
ps::patch_range_within_stack_bounds(start_patch_index, end_patch_index, &patches_vec)
.map_err(RequestReviewBranchError::PatchIndexRangeOutOfBounds)?;
.map_err(BranchError::PatchIndexRangeOutOfBounds)?;

// fetch computed state from Git tree
let patch_stack_base_commit = patch_stack
.base
.peel_to_commit()
.map_err(|_| RequestReviewBranchError::PatchStackBaseNotFound)?;
.map_err(|_| BranchError::PatchStackBaseNotFound)?;

let head_ref_name = patch_stack
.head
.shorthand()
.ok_or(RequestReviewBranchError::PatchStackHeadNoName)?;
.ok_or(BranchError::PatchStackHeadNoName)?;

let patch_info_collection: HashMap<Uuid, state_computation::PatchGitInfo> =
state_computation::get_list_patch_info(repo, patch_stack_base_commit.id(), head_ref_name)
.map_err(RequestReviewBranchError::GetListPatchInfoFailed)?;
.map_err(BranchError::GetListPatchInfoFailed)?;

// collect vector of indexes
let indexes_iter = match end_patch_index {
Expand Down Expand Up @@ -186,25 +186,25 @@ pub fn request_review_branch(
let patch_summary = patch_commit.summary().expect("Patch Missing Summary");
new_branch_name = ps::generate_rr_branch_name(patch_summary);
} else {
return Err(RequestReviewBranchError::PatchSeriesRequireBranchName);
return Err(BranchError::PatchSeriesRequireBranchName);
}
} else if range_patch_branches.len() == 1 {
new_branch_name = range_patch_branches.first().unwrap().to_string()
} else {
return Err(RequestReviewBranchError::AssociatedBranchAmbiguous(
return Err(BranchError::AssociatedBranchAmbiguous(
range_patch_branches.clone(),
));
}

// create branch on top of the patch stack base
let branch = repo
.branch(new_branch_name.as_str(), &patch_stack_base_commit, true)
.map_err(|_| RequestReviewBranchError::CreateRrBranchFailed)?;
.map_err(|_| BranchError::CreateRrBranchFailed)?;

let branch_ref_name = branch
.get()
.name()
.ok_or(RequestReviewBranchError::RrBranchNameNotUtf8)?;
.ok_or(BranchError::RrBranchNameNotUtf8)?;

let start_patch_oid = patches_vec.get(start_patch_index).unwrap().oid;
let start_patch_commit = repo.find_commit(start_patch_oid).unwrap();
Expand Down Expand Up @@ -234,7 +234,7 @@ pub fn request_review_branch(
false,
),
}
.map_err(RequestReviewBranchError::CherryPickFailed)?
.map_err(BranchError::CherryPickFailed)?
.expect("No commits cherry picked, when we expected at least one");

Ok((branch, last_commit_oid_cherry_picked))
Expand Down
2 changes: 1 addition & 1 deletion src/ps/private/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod branch;
pub mod checkout;
pub mod cherry_picking;
pub mod config;
Expand All @@ -6,7 +7,6 @@ pub mod hooks;
pub mod list;
pub mod password_store;
pub mod paths;
pub mod request_review_branch;
pub mod signers;
pub mod state_computation;
pub mod string_file_io;
Expand Down
20 changes: 20 additions & 0 deletions src/ps/public/branch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
use super::super::private;
use super::super::private::git;
use std::result::Result;

#[derive(Debug)]
pub enum BranchError {
OpenRepositoryFailed(git::CreateCwdRepositoryError),
BranchOperationFailed(private::branch::BranchError),
}

pub fn branch(
start_patch_index: usize,
end_patch_index: Option<usize>,
branch_name: Option<String>,
) -> Result<(), BranchError> {
let repo = git::create_cwd_repo().map_err(BranchError::OpenRepositoryFailed)?;
private::branch::branch(&repo, start_patch_index, end_patch_index, branch_name)
.map_err(BranchError::BranchOperationFailed)?;
Ok(())
}
4 changes: 2 additions & 2 deletions src/ps/public/integrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub enum IntegrateError {
PatchAndRemotePatchIdMissmatch(usize),
PatchDiffHashMissmatch(usize),
PatchMissingDiffHash,
CreateOrReplaceBranchFailed(ps::private::request_review_branch::RequestReviewBranchError),
CreateOrReplaceBranchFailed(ps::private::branch::BranchError),
IsolationVerificationFailed(verify_isolation::VerifyIsolationError),
GetPatchBranchNameFailed(git2::Error),
CreatedBranchMissingName,
Expand Down Expand Up @@ -293,7 +293,7 @@ happens.
}

// create/replace the request review branch
let (patch_branch, new_commit_oid) = ps::private::request_review_branch::request_review_branch(
let (patch_branch, new_commit_oid) = ps::private::branch::branch(
&repo,
start_patch_index,
end_patch_index,
Expand Down
2 changes: 1 addition & 1 deletion src/ps/public/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod backup_stack;
pub mod batch_request_review;
pub mod branch;
pub mod checkout;
pub mod fetch;
pub mod integrate;
Expand All @@ -9,7 +10,6 @@ pub mod list;
pub mod pull;
pub mod rebase;
pub mod request_review;
pub mod request_review_branch;
pub mod show;
pub mod sync;
pub mod upstream_patches;
Expand Down
25 changes: 0 additions & 25 deletions src/ps/public/request_review_branch.rs

This file was deleted.

0 comments on commit dbfda73

Please sign in to comment.