From ee91f2824a63e8ba55a5da82076d8a18a0b37410 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Tue, 13 Jun 2023 20:10:26 -0600 Subject: [PATCH 1/2] add more config options for no_merges - option for excluding PRs with certain labels - option for setting labels when merge commits are detected - option for setting a custom comment message --- src/config.rs | 12 +++++++- src/handlers/no_merges.rs | 58 +++++++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/config.rs b/src/config.rs index 3bfcc64e..99f460ee 100644 --- a/src/config.rs +++ b/src/config.rs @@ -94,8 +94,18 @@ pub(crate) struct AssignConfig { #[derive(PartialEq, Eq, Debug, serde::Deserialize)] pub(crate) struct NoMergesConfig { + /// No action will be taken on PRs with these labels. #[serde(default)] - _empty: (), + pub(crate) exclude_labels: Vec, + /// Set these labels on the PR when merge commits are detected. + #[serde(default)] + pub(crate) labels: Vec, + /// Override the default message to post when merge commits are detected. + /// + /// This message will always be followed up with + /// "The following commits are merge commits:" and then + /// a list of the merge commits. + pub(crate) message: Option, } #[derive(PartialEq, Eq, Debug, serde::Deserialize)] diff --git a/src/handlers/no_merges.rs b/src/handlers/no_merges.rs index b9ed89ff..b6b094ae 100644 --- a/src/handlers/no_merges.rs +++ b/src/handlers/no_merges.rs @@ -4,7 +4,7 @@ use crate::{ config::NoMergesConfig, db::issue_data::IssueData, - github::{IssuesAction, IssuesEvent}, + github::{IssuesAction, IssuesEvent, Label}, handlers::Context, }; use anyhow::Context as _; @@ -38,16 +38,23 @@ pub(super) async fn parse_input( return Ok(None); } - // Require an empty configuration block to enable no-merges notifications. - if config.is_none() { + // Require a `[no_merges]` configuration block to enable no-merges notifications. + let Some(config) = config else { return Ok(None); - } + }; // Don't ping on rollups or draft PRs. if event.issue.title.starts_with("Rollup of") || event.issue.draft { return Ok(None); } + // Don't trigger if the PR has any of the excluded labels. + for label in event.issue.labels() { + if config.exclude_labels.contains(&label.name) { + return Ok(None); + } + } + let mut merge_commits = HashSet::new(); let commits = event .issue @@ -73,7 +80,7 @@ pub(super) async fn parse_input( pub(super) async fn handle_input( ctx: &Context, - _config: &NoMergesConfig, + config: &NoMergesConfig, event: &IssuesEvent, input: NoMergesInput, ) -> anyhow::Result<()> { @@ -81,14 +88,9 @@ pub(super) async fn handle_input( let mut state: IssueData<'_, NoMergesState> = IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?; - let since_last_posted = if state.data.mentioned_merge_commits.is_empty() { - "" + let mut message = if let Some(ref message) = config.message { + message.clone() } else { - " (since this message was last posted)" - }; - - let mut should_send = false; - let mut message = format!( " There are merge commits (commits with multiple parents) in your changes. We have a [no merge policy](https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy) so @@ -102,11 +104,25 @@ pub(super) async fn handle_input( $ # delete any merge commits in the editor that appears $ git push --force-with-lease ``` + " + .to_string() + }; + let since_last_posted = if state.data.mentioned_merge_commits.is_empty() { + "" + } else { + " (since this message was last posted)" + }; + write!( + message, + " + The following commits are merge commits{since_last_posted}: + " + ) + .unwrap(); - " - ); + let mut should_send = false; for commit in &input.merge_commits { if state.data.mentioned_merge_commits.contains(commit) { continue; @@ -118,6 +134,20 @@ pub(super) async fn handle_input( } if should_send { + // Set labels + let labels = config + .labels + .iter() + .cloned() + .map(|name| Label { name }) + .collect(); + event + .issue + .add_labels(&ctx.github, labels) + .await + .context("failed to set no_merges labels")?; + + // Post comment event .issue .post_comment(&ctx.github, &message) From a1a20a5e548403568eddf9718ecb93276e202cc8 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Thu, 13 Jul 2023 18:21:02 -0600 Subject: [PATCH 2/2] fixes --- src/handlers/no_merges.rs | 85 +++++++++++++++++++++++++++------------ 1 file changed, 60 insertions(+), 25 deletions(-) diff --git a/src/handlers/no_merges.rs b/src/handlers/no_merges.rs index b6b094ae..e90008cd 100644 --- a/src/handlers/no_merges.rs +++ b/src/handlers/no_merges.rs @@ -78,6 +78,21 @@ pub(super) async fn parse_input( }) } +const DEFAULT_MESSAGE: &str = " +There are merge commits (commits with multiple parents) in your changes. We have a \ +[no merge policy](https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy) \ +so these commits will need to be removed for this pull request to be merged. + +You can start a rebase with the following commands: +```shell-session +$ # rebase +$ git rebase -i master +$ # delete any merge commits in the editor that appears +$ git push --force-with-lease +``` + +"; + pub(super) async fn handle_input( ctx: &Context, config: &NoMergesConfig, @@ -88,37 +103,20 @@ pub(super) async fn handle_input( let mut state: IssueData<'_, NoMergesState> = IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?; - let mut message = if let Some(ref message) = config.message { - message.clone() - } else { - " - There are merge commits (commits with multiple parents) in your changes. We have a - [no merge policy](https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy) so - these commits will need to be removed for this pull request to be merged. - - You can start a rebase with the following commands: - - ```shell-session - $ # rebase - $ git rebase -i master - $ # delete any merge commits in the editor that appears - $ git push --force-with-lease - ``` - " - .to_string() - }; + let mut message = config + .message + .as_deref() + .unwrap_or(DEFAULT_MESSAGE) + .to_string(); let since_last_posted = if state.data.mentioned_merge_commits.is_empty() { "" } else { " (since this message was last posted)" }; - write!( + writeln!( message, - " - - The following commits are merge commits{since_last_posted}: - " + "The following commits are merge commits{since_last_posted}:" ) .unwrap(); @@ -130,7 +128,7 @@ pub(super) async fn handle_input( should_send = true; state.data.mentioned_merge_commits.insert((*commit).clone()); - write!(message, "- {commit}").unwrap(); + writeln!(message, "- {commit}").unwrap(); } if should_send { @@ -157,3 +155,40 @@ pub(super) async fn handle_input( } Ok(()) } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn message() { + let mut message = DEFAULT_MESSAGE.to_string(); + + writeln!(message, "The following commits are merge commits:").unwrap(); + + for n in 1..5 { + writeln!(message, "- commit{n}").unwrap(); + } + + assert_eq!( + message, + " +There are merge commits (commits with multiple parents) in your changes. We have a [no merge policy](https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy) so these commits will need to be removed for this pull request to be merged. + +You can start a rebase with the following commands: +```shell-session +$ # rebase +$ git rebase -i master +$ # delete any merge commits in the editor that appears +$ git push --force-with-lease +``` + +The following commits are merge commits: +- commit1 +- commit2 +- commit3 +- commit4 +" + ); + } +}