Skip to content

Commit

Permalink
no-merges: match titles instead of labels
Browse files Browse the repository at this point in the history
also don't re-add labels if they're manually removed

labels are not always set atomically when opening a PR
example: rust-lang/miri#3059
  • Loading branch information
pitaj committed Sep 29, 2023
1 parent ebce000 commit f106af7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ impl AssignConfig {

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct NoMergesConfig {
/// No action will be taken on PRs with these labels.
/// No action will be taken on PRs with these substrings in the title.
#[serde(default)]
pub(crate) exclude_labels: Vec<String>,
pub(crate) exclude_titles: Vec<String>,
/// Set these labels on the PR when merge commits are detected.
#[serde(default)]
pub(crate) labels: Vec<String>,
Expand Down
42 changes: 30 additions & 12 deletions src/handlers/no_merges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ pub(super) async fn parse_input(
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);
}
// Don't trigger if the PR has any of the excluded title segments.
if config
.exclude_titles
.iter()
.any(|s| event.issue.title.contains(s))
{
return Ok(None);
}

let mut merge_commits = HashSet::new();
Expand All @@ -70,12 +72,11 @@ pub(super) async fn parse_input(
}
}

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

Ok(Some(NoMergesInput { merge_commits }))
}

const DEFAULT_MESSAGE: &str = "
Expand All @@ -102,14 +103,15 @@ pub(super) async fn handle_input(
let mut client = ctx.db.get().await;
let mut state: IssueData<'_, NoMergesState> =
IssueData::load(&mut client, &event.issue, NO_MERGES_KEY).await?;
let first_time = state.data.mentioned_merge_commits.is_empty();

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() {
let since_last_posted = if first_time {
""
} else {
" (since this message was last posted)"
Expand All @@ -132,6 +134,22 @@ pub(super) async fn handle_input(
}

if should_send {
if !first_time {
// Check if the labels are still set.
// Otherwise, they were probably removed manually.
let any_removed = config.labels.iter().any(|label| {
// No label on the issue matches.
event.issue.labels().iter().all(|l| &l.name != label)
});

if any_removed {
// Assume it was a false positive, so don't
// re-add the labels or send a message this time.
state.save().await?;
return Ok(());
}
}

// Set labels
let labels = config
.labels
Expand Down

0 comments on commit f106af7

Please sign in to comment.