Skip to content

Commit

Permalink
Add opts to disable hooks to request-review cmd
Browse files Browse the repository at this point in the history
Add opts to disable hooks to request-review command so that users can
disable the hooks. Specifically, I added the --no-post-sync-hook and
--no-isolation-verification-hook options. Now that these options exist
the request-review command is able to do everything that sync command
could do and more. This will let us in the future remove the sync
command and collapse the command service area.

added: --no-isolation-verification-hook opt to request-review
added: --no-post-sync-hook opt to request-review

<!-- ps-id: ddc0e041-77b1-427c-94e1-4a63928e68ff -->
  • Loading branch information
drewdeponte committed Oct 20, 2023
1 parent dbfda73 commit fb7d061
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 15 deletions.
6 changes: 6 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ pub struct RequestReview {
/// Use the provided branch name instead of generating one
#[arg(short = 'n')]
pub branch_name: Option<String>,
/// disable isolation verification hook
#[arg(long = "no-isolation-verification-hook", action(ArgAction::SetFalse))]
pub isolation_verification_hook: bool,
/// disable post sync hook
#[arg(long = "no-post-sync-hook", action(ArgAction::SetFalse))]
pub post_sync_hook: bool,
}

#[derive(Debug, Args)]
Expand Down
10 changes: 9 additions & 1 deletion src/commands/request_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,22 @@ use super::utils::print_err;
use gps as ps;
use std::str::FromStr;

pub fn request_review(patch_index_or_range: String, branch_name: Option<String>, color: bool) {
pub fn request_review(
patch_index_or_range: String,
branch_name: Option<String>,
color: bool,
isolation_verification_hook: bool,
post_sync_hook: bool,
) {
match PatchIndexRange::from_str(&patch_index_or_range) {
Ok(patch_index_range) => {
match ps::request_review(
patch_index_range.start_index,
patch_index_range.end_index,
branch_name,
color,
isolation_verification_hook,
post_sync_hook,
) {
Ok(_) => {}
Err(ps::RequestReviewError::PostSyncHookNotFound) => print_err(
Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ fn main() {
opts.patch_index_or_range,
opts.branch_name,
cli.color,
opts.isolation_verification_hook,
opts.post_sync_hook,
),
cli::Command::BatchRequestReview(opts) => {
commands::batch_request_review::batch_request_review(opts.patch_index, cli.color)
Expand Down
2 changes: 1 addition & 1 deletion src/ps/public/batch_request_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn batch_request_review(
) -> Result<(), BatchRequestReviewError> {
let mut errors: Vec<request_review::RequestReviewError> = Vec::new();
for patch_index in patch_indexes {
match request_review::request_review(patch_index, None, None, color) {
match request_review::request_review(patch_index, None, None, color, true, true) {
Ok(_) => {}
Err(e) => errors.push(e),
};
Expand Down
30 changes: 17 additions & 13 deletions src/ps/public/request_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ pub fn request_review(
end_patch_index: Option<usize>,
given_branch_name: Option<String>,
color: bool,
isolation_verification_hook: bool,
post_sync_hook: bool,
) -> Result<(), RequestReviewError> {
let repo = git::create_cwd_repo().map_err(RequestReviewError::OpenRepositoryFailed)?;

Expand All @@ -113,7 +115,7 @@ pub fn request_review(
.map_err(RequestReviewError::GetConfigFailed)?;

// verify isolation
if config.request_review.verify_isolation {
if isolation_verification_hook && config.request_review.verify_isolation {
verify_isolation::verify_isolation(start_patch_index, end_patch_index, color)
.map_err(RequestReviewError::IsolationVerificationFailed)?;
}
Expand Down Expand Up @@ -150,18 +152,20 @@ pub fn request_review(
let cur_patch_stack_upstream_branch_name_relative_to_remote =
str::replace(&cur_patch_stack_upstream_branch_name, pattern.as_str(), "");

utils::execute(
request_review_post_sync_hook_path
.to_str()
.ok_or(RequestReviewError::PathNotUtf8)?,
&[
&patch_upstream_branch_name,
&cur_patch_stack_upstream_branch_name_relative_to_remote,
cur_patch_stack_upstream_branch_remote_name_str,
cur_patch_stack_upstream_branch_remote_url_str,
],
)
.map_err(RequestReviewError::HookExecutionFailed)?;
if post_sync_hook {
utils::execute(
request_review_post_sync_hook_path
.to_str()
.ok_or(RequestReviewError::PathNotUtf8)?,
&[
&patch_upstream_branch_name,
&cur_patch_stack_upstream_branch_name_relative_to_remote,
cur_patch_stack_upstream_branch_remote_name_str,
cur_patch_stack_upstream_branch_remote_url_str,
],
)
.map_err(RequestReviewError::HookExecutionFailed)?;
}

Ok(())
}

0 comments on commit fb7d061

Please sign in to comment.