Skip to content

Commit

Permalink
Add request_review_post_sync hook messaging
Browse files Browse the repository at this point in the history
Add specific messaging around the request_review_post_sync hook as it is
a blocker for the requset-review command to function. So I wanted to
make it clear to the user and give them information about it and where
they can go to find more information. I also wanted to make the hook
found but not executable case well known and how to address it easy for
the user as it as well is blocker for the request-review command
functioning properly.

This relates it issue #80.

[changelog]
added: request_review_post_sync messaging in requset-review command

ps-id: 81428726-0981-41cb-9318-cf76eb849edc
  • Loading branch information
drewdeponte committed May 4, 2022
1 parent 1e5746e commit bfbe489
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ pub mod status;
pub mod add_changes_to_stage;
pub mod log;
pub mod unstage;
pub mod utils;
55 changes: 46 additions & 9 deletions src/commands/request_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,55 @@
// inside of the `ps` module.

use gps as ps;
use ansi_term::Colour::Red;
use super::utils::print_err;

pub fn request_review(patch_index: usize, branch_name: Option<String>, color: bool) {
match ps::request_review(patch_index, branch_name) {
Ok(_) => {},
Err(e) => {
let error_string = format!("\n\n{}\n\n", e);
if color {
eprintln!("{}", Red.paint(error_string))
} else {
eprintln!("{}", error_string)
}
}
Err(ps::RequestReviewError::PostSyncHookNotFound) => {
print_err(color,
r#"
The request_review_post_sync hook was not found!
This hook is required to be installed and configured for the
request-review command. It is executed after the patch is successfully
sync'd to the remote.
It is responsible for requesting review of the patch within your
preferred code review platform & workflow. For example if you worked
on the Git or Linux Kernel dev teams you might want it to format your
patch as an email and send it to the appropriate mailing list. If on
the other hand you use GitHub and pull requests for code reviews you
could have it simply create the pull request for you on GitHub.
You can effectively have it do whatever you want as it is just a hook.
An exit status of 0, success, informs gps that it should update it's
state tracking for that patch to indicate that it has been requested
for review. Any non-zero exit status will indicate failure and cause
gps to abort and not update the patch's state.
You can find more information and examples of this hook and others at
the following.
https://github.com/uptech/git-ps-rs#hooks
"#)
},
Err(ps::RequestReviewError::PostSyncHookNotExecutable(path)) => {
let path_str = path.to_str().unwrap_or("unknow path");
let msg = format!(
r#"
The request_review_post_sync hook was found at
{}
but it is NOT executable. For the request-review command to function properly
the hook needs to be executable. Generally this can be done with the
following.
chmod u+x {}
"#, path_str, path_str);
print_err(color, &msg);
},
Err(e) => print_err(color, format!("\n{}\n", e).as_str())
};
}
10 changes: 10 additions & 0 deletions src/commands/utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
use ansi_term::Colour::Red;

pub fn print_err(color: bool, message: &str) {
if color {
eprintln!("{}", Red.paint(message))
} else {
eprintln!("{}", message)
}
}

2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use ps::public::sync::{sync, SyncError};
pub use ps::public::branch::branch;
pub use ps::public::list::list;
pub use ps::public::rebase::rebase;
pub use ps::public::request_review::request_review;
pub use ps::public::request_review::{request_review, RequestReviewError};
pub use ps::public::show::show;
pub use ps::public::integrate;
pub use ps::public::checkout::checkout;
Expand Down
21 changes: 18 additions & 3 deletions src/ps/public/request_review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ use super::super::private::config;
use super::super::private::verify_isolation;
use std::result::Result;
use std::fmt;
use std::path::PathBuf;

#[derive(Debug)]
pub enum RequestReviewError {
OpenRepositoryFailed(git::CreateCwdRepositoryError),
GetRepoRootPathFailed(paths::PathsError),
PathNotUtf8,
BranchNameNotUtf8,
HookNotFound(hooks::FindHookError),
PostSyncHookNotFound,
PostSyncHookNotExecutable(PathBuf),
FindHookFailed(hooks::FindHookError),
SyncFailed(ps::public::sync::SyncError),
FetchPatchMetaDataFailed(state_management::FetchPatchMetaDataError),
PatchMetaDataMissing,
Expand All @@ -30,14 +33,26 @@ pub enum RequestReviewError {
PatchCommitDiffPatchIdFailed(git::CommitDiffPatchIdError)
}

impl From<hooks::FindHookError> for RequestReviewError {
fn from(e: hooks::FindHookError) -> Self {
match e {
hooks::FindHookError::NotFound => Self::PostSyncHookNotFound,
hooks::FindHookError::NotExecutable(path) => Self::PostSyncHookNotExecutable(path),
hooks::FindHookError::PathExpandHomeFailed(_) => Self::FindHookFailed(e)
}
}
}

impl fmt::Display for RequestReviewError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::OpenRepositoryFailed(e) => write!(f, "Repository not found in current working directory - {:?}", e),
Self::GetRepoRootPathFailed(e) => write!(f, "Get repository path failed - {:?}", e),
Self::PathNotUtf8 => write!(f, "Failed to process repository root path as it is NOT utf8"),
Self::BranchNameNotUtf8 => write!(f, "Failed to process remote name as it is NOT utf8"),
Self::HookNotFound(e) => write!(f, "request_review_post_sync hook not found - {:?}", e),
Self::PostSyncHookNotFound => write!(f, "request_review_post_sync hook not found"),
Self::PostSyncHookNotExecutable(path) => write!(f, "request_review_post_sync hook - {} - is not executable", path.to_str().unwrap_or("unknown path")),
Self::FindHookFailed(e) => write!(f, "failed to find request_review_post_sync hook - {:?}", e),
Self::SyncFailed(e) => write!(f, "Failed to sync patch to remote - {:?}", e),
Self::FetchPatchMetaDataFailed(e) => write!(f, "Failed to fetch patch meta data - {:?}", e),
Self::PatchMetaDataMissing => write!(f, "Patch meta data unexpectedly missing"),
Expand All @@ -64,7 +79,7 @@ pub fn request_review(patch_index: usize, given_branch_name: Option<String>) ->
// find post_request_review hook
let repo_root_path = paths::repo_root_path(&repo).map_err(RequestReviewError::GetRepoRootPathFailed)?;
let repo_root_str = repo_root_path.to_str().ok_or(RequestReviewError::PathNotUtf8)?;
let hook_path = hooks::find_hook(repo_root_str, "request_review_post_sync").map_err(RequestReviewError::HookNotFound)?;
let hook_path = hooks::find_hook(repo_root_str, "request_review_post_sync")?;

let config = config::get_config(repo_root_str).map_err(RequestReviewError::GetConfigFailed)?;

Expand Down

0 comments on commit bfbe489

Please sign in to comment.