From 2d95522a237514497c123df387074724d7825a9a Mon Sep 17 00:00:00 2001 From: Urgau Date: Sun, 24 Nov 2024 13:29:09 +0100 Subject: [PATCH] Add support for exceptions in non default branch warning --- src/config.rs | 105 +++++++++++++++++++++++++++++++++++++++-- src/handlers/assign.rs | 29 +++++++++--- 2 files changed, 124 insertions(+), 10 deletions(-) diff --git a/src/config.rs b/src/config.rs index 523836fb..6ed913a1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -90,10 +90,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, /// Ad-hoc groups that can be referred to in `owners`. @@ -117,6 +117,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, + }, +} + +#[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 { @@ -501,7 +540,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(), @@ -530,4 +569,64 @@ 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).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, + } + ); + } } diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index 3a91e501..e6e67306 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -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, @@ -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 { @@ -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() { @@ -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 { +/// 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 { 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), )