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

handlers: add no merge policy notifications #1642

Merged
merged 1 commit into from Oct 4, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/config.rs
Expand Up @@ -33,6 +33,7 @@ pub(crate) struct Config {
pub(crate) shortcut: Option<ShortcutConfig>,
pub(crate) note: Option<NoteConfig>,
pub(crate) mentions: Option<MentionsConfig>,
pub(crate) no_merges: Option<NoMergesConfig>,
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
Expand Down Expand Up @@ -79,6 +80,12 @@ pub(crate) struct AssignConfig {
_empty: (),
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct NoMergesConfig {
#[serde(default)]
_empty: (),
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct NoteConfig {
#[serde(default)]
Expand Down Expand Up @@ -365,6 +372,7 @@ mod tests {
github_releases: None,
review_submitted: None,
mentions: None,
no_merges: None,
}
);
}
Expand Down
27 changes: 27 additions & 0 deletions src/github.rs
Expand Up @@ -749,6 +749,33 @@ impl Issue {
Ok(Some(String::from(String::from_utf8_lossy(&diff))))
}

/// Returns the commits from this pull request (no commits are returned if this `Issue` is not
/// a pull request).
pub async fn commits(&self, client: &GithubClient) -> anyhow::Result<Vec<GithubCommit>> {
if !self.is_pr() {
return Ok(vec![]);
}

let mut commits = Vec::new();
let mut page = 1;
loop {
let req = client.get(&format!(
"{}/pulls/{}/commits?page={page}&per_page=100",
self.repository().url(),
self.number
));

let new: Vec<_> = client.json(req).await?;
if new.is_empty() {
break;
}
commits.extend(new);

page += 1;
}
Ok(commits)
}

pub async fn files(&self, client: &GithubClient) -> anyhow::Result<Vec<PullRequestFile>> {
if !self.is_pr() {
return Ok(vec![]);
Expand Down
2 changes: 2 additions & 0 deletions src/handlers.rs
Expand Up @@ -31,6 +31,7 @@ mod glacier;
mod major_change;
mod mentions;
mod milestone_prs;
mod no_merges;
mod nominate;
mod note;
mod notification;
Expand Down Expand Up @@ -155,6 +156,7 @@ issue_handlers! {
autolabel,
major_change,
mentions,
no_merges,
notify_zulip,
}

Expand Down
129 changes: 129 additions & 0 deletions src/handlers/no_merges.rs
@@ -0,0 +1,129 @@
//! Purpose: When opening a PR, or pushing new changes, check for merge commits
//! and notify the user of our no-merge policy.

use crate::{
config::NoMergesConfig,
db::issue_data::IssueData,
github::{IssuesAction, IssuesEvent},
handlers::Context,
};
use anyhow::Context as _;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use std::fmt::Write;
use tracing as log;

const NO_MERGES_KEY: &str = "no_merges";

pub(super) struct NoMergesInput {
/// Hashes of merge commits in the pull request.
merge_commits: HashSet<String>,
}

#[derive(Debug, Default, Deserialize, Serialize)]
struct NoMergesState {
/// Hashes of merge commits that have already been mentioned by triagebot in a comment.
mentioned_merge_commits: HashSet<String>,
}

pub(super) async fn parse_input(
ctx: &Context,
event: &IssuesEvent,
config: Option<&NoMergesConfig>,
) -> Result<Option<NoMergesInput>, String> {
if !matches!(
event.action,
IssuesAction::Opened | IssuesAction::Synchronize | IssuesAction::ReadyForReview
) {
return Ok(None);
}

// Require an empty configuration block to enable no-merges notifications.
if config.is_none() {
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);
}

let mut merge_commits = HashSet::new();
let commits = event
.issue
.commits(&ctx.github)
.await
.map_err(|e| {
log::error!("failed to fetch commits: {:?}", e);
})
.unwrap_or_default();
for commit in commits {
if commit.parents.len() > 1 {
merge_commits.insert(commit.sha.clone());
}
}

let input = NoMergesInput { merge_commits };
Ok(if input.merge_commits.is_empty() {
None
} else {
Some(input)
})
}

pub(super) async fn handle_input(
ctx: &Context,
_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 since_last_posted = if state.data.mentioned_merge_commits.is_empty() {
""
} 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
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();
}

if should_send {
event
.issue
.post_comment(&ctx.github, &message)
.await
.context("failed to post no_merges comment")?;
state.save().await?;
}
Ok(())
}