Skip to content

Commit

Permalink
Add support for exceptions in non default branch warning
Browse files Browse the repository at this point in the history
  • Loading branch information
Urgau committed Nov 24, 2024
1 parent d7d4bab commit 56928c7
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 10 deletions.
106 changes: 103 additions & 3 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ pub(crate) struct PingTeamConfig {
#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct AssignConfig {
/// If `true`, then posts a warning comment if the PR is opened against a
/// If enabled, then posts a warning comment if the PR is opened against a
/// different branch than the default (usually master or main).
#[serde(default)]
pub(crate) warn_non_default_branch: bool,
pub(crate) warn_non_default_branch: WarnNonDefaultBranchConfig,
/// A URL to include in the welcome message.
pub(crate) contributing_url: Option<String>,
/// Ad-hoc groups that can be referred to in `owners`.
Expand All @@ -118,6 +118,45 @@ impl AssignConfig {
}
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(deny_unknown_fields)]
#[serde(untagged)]
pub(crate) enum WarnNonDefaultBranchConfig {
Simple(bool),
Extended {
enable: bool,
/// List of exceptions that have a different default branch
#[serde(default)]
exceptions: Vec<WarnNonDefaultBranchException>,
},
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct WarnNonDefaultBranchException {
/// Substring in the title that match this exception
pub(crate) title: String,
/// The actual branch that should be associated with the issue
pub(crate) branch: String,
}

impl Default for WarnNonDefaultBranchConfig {
fn default() -> WarnNonDefaultBranchConfig {
WarnNonDefaultBranchConfig::Simple(false)
}
}

impl WarnNonDefaultBranchConfig {
pub(crate) fn enabled_and_exceptions(&self) -> Option<&[WarnNonDefaultBranchException]> {
match self {
WarnNonDefaultBranchConfig::Simple(enable) => enable.then(|| &[] as &[_]),
WarnNonDefaultBranchConfig::Extended { enable, exceptions } => {
enable.then(|| exceptions.as_slice())
}
}
}
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub(crate) struct NoMergesConfig {
Expand Down Expand Up @@ -514,7 +553,7 @@ mod tests {
allow_unauthenticated: vec!["C-*".into()],
}),
assign: Some(AssignConfig {
warn_non_default_branch: false,
warn_non_default_branch: WarnNonDefaultBranchConfig::Simple(false),
contributing_url: None,
adhoc_groups: HashMap::new(),
owners: HashMap::new(),
Expand Down Expand Up @@ -544,4 +583,65 @@ mod tests {
}
);
}

#[test]
fn warn_non_default_branch() {
let config = r#"
[assign]
warn_non_default_branch.enable = true
[[assign.warn_non_default_branch.exceptions]]
title = "[beta"
branch = "beta"
[[assign.warn_non_default_branch.exceptions]]
title = "[stable"
branch = "stable"
"#;
let config = toml::from_str::<Config>(&config).unwrap();
assert_eq!(
config,
Config {
relabel: None,
assign: Some(AssignConfig {
warn_non_default_branch: WarnNonDefaultBranchConfig::Extended {
enable: true,
exceptions: vec![
WarnNonDefaultBranchException {
title: "[beta".to_string(),
branch: "beta".to_string()
},
WarnNonDefaultBranchException {
title: "[stable".to_string(),
branch: "stable".to_string()
},
],
},
contributing_url: None,
adhoc_groups: HashMap::new(),
owners: HashMap::new(),
users_on_vacation: HashSet::new(),
}),
note: None,
ping: None,
nominate: None,
shortcut: None,
prioritize: None,
major_change: None,
glacier: None,
close: None,
autolabel: None,
notify_zulip: None,
github_releases: None,
review_submitted: None,
review_requested: None,
mentions: None,
no_merges: None,
validate_config: Some(ValidateConfig {}),
pr_tracking: None,
transfer: None,
merge_conflicts: None,
}
);
}
}
29 changes: 22 additions & 7 deletions src/handlers/assign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! the PR modifies.
use crate::{
config::AssignConfig,
config::{AssignConfig, WarnNonDefaultBranchException},
github::{self, Event, FileDiff, Issue, IssuesAction, Selection},
handlers::{Context, GithubClient, IssuesEvent},
interactions::EditIssueBody,
Expand Down Expand Up @@ -73,6 +73,11 @@ const NON_DEFAULT_BRANCH: &str =
but this one is against {target}. \
Please double check that you specified the right target!";

const NON_DEFAULT_BRANCH_EXCEPTION: &str =
"Pull requests targetting the {default} branch are usually filed against the {default} \
branch, but this one is against {target}. \
Please double check that you specified the right target!";

const SUBMODULE_WARNING_MSG: &str = "These commits modify **submodules**.";

fn on_vacation_msg(user: &str) -> String {
Expand Down Expand Up @@ -179,8 +184,8 @@ pub(super) async fn handle_input(

// Compute some warning messages to post to new PRs.
let mut warnings = Vec::new();
if config.warn_non_default_branch {
warnings.extend(non_default_branch(event));
if let Some(exceptions) = config.warn_non_default_branch.enabled_and_exceptions() {
warnings.extend(non_default_branch(exceptions, event));
}
warnings.extend(modifies_submodule(diff));
if !warnings.is_empty() {
Expand Down Expand Up @@ -209,15 +214,25 @@ fn is_self_assign(assignee: &str, pr_author: &str) -> bool {
assignee.to_lowercase() == pr_author.to_lowercase()
}

/// Returns a message if the PR is opened against the non-default branch.
fn non_default_branch(event: &IssuesEvent) -> Option<String> {
/// Returns a message if the PR is opened against the non-default branch (or the exception branch
/// if it's an exception).
fn non_default_branch(
exceptions: &[WarnNonDefaultBranchException],
event: &IssuesEvent,
) -> Option<String> {
let target_branch = &event.issue.base.as_ref().unwrap().git_ref;
let default_branch = &event.repository.default_branch;
let (default_branch, warn_msg) = exceptions
.iter()
.find(|e| event.issue.title.contains(&e.title))
.map_or_else(
|| (&event.repository.default_branch, NON_DEFAULT_BRANCH),
|e| (&e.branch, NON_DEFAULT_BRANCH_EXCEPTION),
);
if target_branch == default_branch {
return None;
}
Some(
NON_DEFAULT_BRANCH
warn_msg
.replace("{default}", default_branch)
.replace("{target}", target_branch),
)
Expand Down

0 comments on commit 56928c7

Please sign in to comment.