Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more config options for no_merges #1704

Merged
merged 2 commits into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
/// Set these labels on the PR when merge commits are detected.
#[serde(default)]
pub(crate) labels: Vec<String>,
/// 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<String>,
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
Expand Down
115 changes: 90 additions & 25 deletions src/handlers/no_merges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 _;
Expand Down Expand Up @@ -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
Expand All @@ -71,53 +78,74 @@ 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,
config: &NoMergesConfig,
event: &IssuesEvent,
input: NoMergesInput,
) -> anyhow::Result<()> {
let mut client = ctx.db.get().await;
let mut state: IssueData<'_, NoMergesState> =
IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?;

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)"
};
writeln!(
message,
"The following commits are merge commits{since_last_posted}:"
)
.unwrap();

let mut should_send = false;
pitaj marked this conversation as resolved.
Show resolved Hide resolved
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
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{since_last_posted}:

"
);
for commit in &input.merge_commits {
if state.data.mentioned_merge_commits.contains(commit) {
continue;
}

should_send = true;
state.data.mentioned_merge_commits.insert((*commit).clone());
write!(message, "- {commit}").unwrap();
writeln!(message, "- {commit}").unwrap();
}

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)
Expand All @@ -127,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
"
);
}
}