Skip to content
This repository has been archived by the owner on Oct 11, 2023. It is now read-only.

Commit

Permalink
do not check for companion's status upfront since they're expected to…
Browse files Browse the repository at this point in the history 
… be failing at first
  • Loading branch information
@joao-paulo-parity
joao-paulo-parity committed Oct 15, 2021
1 parent c06b24a commit 271f265
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 204 deletions.
118 changes: 10 additions & 108 deletions src/companion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ use crate::{
github::*,
github_bot::GithubBot,
webhook::{
check_merge_is_allowed, cleanup_merged_pr, get_latest_checks_state,
get_latest_statuses_state, merge, ready_to_merge, wait_to_merge,
AppState, MergeRequest,
check_merge_is_allowed, cleanup_merged_pr, merge, ready_to_merge,
wait_to_merge, AppState, MergeRequest,
},
MergeAllowedOutcome, Result, Status, COMPANION_LONG_REGEX,
COMPANION_PREFIX_REGEX, COMPANION_SHORT_REGEX, PR_HTML_URL_REGEX,
MergeAllowedOutcome, Result, COMPANION_LONG_REGEX, COMPANION_PREFIX_REGEX,
COMPANION_SHORT_REGEX, PR_HTML_URL_REGEX,
};

async fn update_companion_repository(
Expand Down Expand Up @@ -475,66 +474,6 @@ pub async fn check_all_companions_are_mergeable(
}
_ => (),
}

match get_latest_statuses_state(
github_bot,
&companion.base.repo.owner.login,
&companion.base.repo.name,
&companion.head.sha,
&companion.html_url,
)
.await?
{
Status::Success => (),
Status::Pending => {
return Err(Error::InvalidCompanionStatus {
value: InvalidCompanionStatusValue::Pending,
msg: format!(
"Companion {} has pending statuses",
companion.html_url
),
});
}
Status::Failure => {
return Err(Error::InvalidCompanionStatus {
value: InvalidCompanionStatusValue::Failure,
msg: format!(
"Companion {} has failed statuses",
companion.html_url
),
});
}
};

match get_latest_checks_state(
github_bot,
&companion.base.repo.owner.login,
&companion.base.repo.name,
&companion.head.sha,
&companion.html_url,
)
.await?
{
Status::Success => (),
Status::Pending => {
return Err(Error::InvalidCompanionStatus {
value: InvalidCompanionStatusValue::Pending,
msg: format!(
"Companion {} has pending checks",
companion.html_url
),
});
}
Status::Failure => {
return Err(Error::InvalidCompanionStatus {
value: InvalidCompanionStatusValue::Failure,
msg: format!(
"Companion {} has failed checks",
companion.html_url
),
});
}
};
}

Ok(())
Expand All @@ -557,18 +496,8 @@ async fn update_then_merge_companion(
return Ok(());
}

if let Err(err) =
check_merge_is_allowed(state, &companion, requested_by, None, &vec![])
.await
{
match err {
Error::InvalidCompanionStatus { ref value, .. } => match value {
InvalidCompanionStatusValue::Pending => (),
InvalidCompanionStatusValue::Failure => return Err(err),
},
err => return Err(err),
};
}
check_merge_is_allowed(state, &companion, requested_by, None, &vec![])
.await?;

log::info!("Updating companion {}", html_url);
let updated_sha = update_companion_repository(
Expand All @@ -589,28 +518,10 @@ async fn update_then_merge_companion(
// failed already
let companion = github_bot.pull_request(&owner, &repo, *number).await?;

let should_wait_for_companions = match check_merge_is_allowed(
state,
&companion,
requested_by,
None,
&vec![],
)
.await
{
Ok(_) => false,
Err(err) => match err {
Error::InvalidCompanionStatus { ref value, .. } => match value {
InvalidCompanionStatusValue::Pending => true,
InvalidCompanionStatusValue::Failure => return Err(err),
},
err => return Err(err),
},
};
check_merge_is_allowed(state, &companion, requested_by, None, &vec![])
.await?;

if !should_wait_for_companions
&& ready_to_merge(&state.github_bot, &companion).await?
{
if ready_to_merge(&state.github_bot, &companion).await? {
if let Err(err) = merge(state, &companion, requested_by, None).await? {
return match err {
Error::MergeFailureWillBeSolvedLater { .. } => Ok(()),
Expand All @@ -631,15 +542,6 @@ async fn update_then_merge_companion(

let companion_children = companion.parse_all_mr_base(&vec![]);

let msg = if let Some(true) = companion_children
.as_ref()
.map(|children| !children.is_empty())
{
Some("Waiting for companions' statuses and this PR's statuses")
} else {
None
};

wait_to_merge(
state,
&updated_sha,
Expand All @@ -651,7 +553,7 @@ async fn update_then_merge_companion(
requested_by: requested_by.to_owned(),
companion_children: companion_children,
},
msg,
None,
)
.await?;
}
Expand Down
12 changes: 0 additions & 12 deletions src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ pub struct CompanionDetailsWithErrorMessage {
pub msg: String,
}

#[derive(Debug, PartialEq)]
pub enum InvalidCompanionStatusValue {
Pending,
Failure,
}

#[derive(Debug, Snafu)]
#[snafu(visibility = "pub")]
pub enum Error {
Expand Down Expand Up @@ -187,12 +181,6 @@ pub enum Error {
msg: String,
},

#[snafu(display("{}", msg))]
InvalidCompanionStatus {
value: InvalidCompanionStatusValue,
msg: String,
},

#[snafu(display("Failed to merge companions: {:?}", errors))]
CompanionsFailedMerge {
errors: Vec<CompanionDetailsWithErrorMessage>,
Expand Down
92 changes: 8 additions & 84 deletions src/webhook.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,19 +547,7 @@ async fn checks_and_status(state: &AppState, sha: &str) -> Result<()> {
return Ok(());
}

if let Err(err) =
check_merge_is_allowed(state, &pr, &requested_by, None, &vec![]).await
{
return match err {
Error::InvalidCompanionStatus { ref value, .. } => {
match value {
InvalidCompanionStatusValue::Pending => Ok(()),
InvalidCompanionStatusValue::Failure => Err(err),
}
}
err => Err(err),
};
}
check_merge_is_allowed(state, &pr, &requested_by, None, &vec![]).await?;

if let Some((parent_sha, parent)) = parent.as_ref() {
let parent_pr = github_bot
Expand All @@ -579,27 +567,14 @@ async fn checks_and_status(state: &AppState, sha: &str) -> Result<()> {

if is_still_companion && !parent_pr.merged {
let was_parent_merged = async {
if let Err(err) = check_merge_is_allowed(
check_merge_is_allowed(
state,
&parent_pr,
&requested_by,
None,
&vec![]
)
.await
{
return match err {
Error::InvalidCompanionStatus { ref value, .. } if *value == InvalidCompanionStatusValue::Pending => {
log::info!(
"Skipped merging {} because parent {} had pending companion statuses",
pr.html_url,
parent_pr.html_url
);
Ok(false)
},
_ => Err(err)
}
}
.await?;

if !ready_to_merge(github_bot, &parent_pr).await? {
log::info!(
Expand Down Expand Up @@ -741,22 +716,7 @@ async fn checks_and_status(state: &AppState, sha: &str) -> Result<()> {
}
}

if let Err(err) =
check_merge_is_allowed(state, &pr, &requested_by, None, &vec![]).await
{
return match err {
Error::InvalidCompanionStatus { ref value, .. } => {
match value {
InvalidCompanionStatusValue::Pending => {
log::info!("In checks_and_status, skipped merging {} because it had companions pending", pr.html_url);
Ok(())
},
InvalidCompanionStatusValue::Failure => Err(err),
}
}
err => Err(err),
};
};
check_merge_is_allowed(state, &pr, &requested_by, None, &vec![]).await?;

if let Err(err) = merge(state, &pr, &requested_by, None).await? {
return match err {
Expand Down Expand Up @@ -832,38 +792,12 @@ async fn handle_command(
companion_children: pr.parse_all_mr_base(&vec![]),
};

let should_wait_for_companions = match check_merge_is_allowed(
state,
&pr,
requested_by,
None,
&vec![],
)
.await
{
Ok(_) => false,
Err(err) => match err {
Error::InvalidCompanionStatus { ref value, .. } => {
match value {
InvalidCompanionStatusValue::Pending => match cmd {
MergeCommentCommand::Normal => true,
MergeCommentCommand::Force => false,
},
InvalidCompanionStatusValue::Failure => match cmd {
MergeCommentCommand::Normal => return Err(err),
MergeCommentCommand::Force => false,
},
}
}
_ => return Err(err),
},
};
check_merge_is_allowed(state, &pr, requested_by, None, &vec![])
.await?;

match cmd {
MergeCommentCommand::Normal => {
if !should_wait_for_companions
&& ready_to_merge(github_bot, &pr).await?
{
if ready_to_merge(github_bot, &pr).await? {
match merge(state, pr, requested_by, None).await? {
// If the merge failure will be solved later, then register the PR in the database so that
// it'll eventually resume processing when later statuses arrive
Expand Down Expand Up @@ -891,17 +825,7 @@ async fn handle_command(
_ => (),
}
} else {
wait_to_merge(
state,
&pr.head.sha,
&mr,
if should_wait_for_companions {
Some("Waiting for companions' statuses and this PR's statuses")
} else {
None
},
)
.await?;
wait_to_merge(state, &pr.head.sha, &mr, None).await?;
return Ok(());
}
}
Expand Down

0 comments on commit 271f265

Please sign in to comment.