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

no-merges: match titles instead of labels #1720

Merged
merged 1 commit into from
Oct 15, 2023

Conversation

pitaj
Copy link
Contributor

@pitaj pitaj commented Sep 16, 2023

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

Forge change: rust-lang/rust-forge#701

pitaj added a commit to pitaj/rust-forge that referenced this pull request Sep 16, 2023
@pitaj pitaj force-pushed the no-merges_exclude-titles branch from f8d45c0 to a27cc0f Compare September 16, 2023 03:13
bors added a commit to rust-lang/miri that referenced this pull request Sep 16, 2023
bors added a commit to rust-lang/miri that referenced this pull request Sep 21, 2023
disable no-merges check for now

It leads to false warnings on sync PRs until rust-lang/triagebot#1720 lands.
bors added a commit to rust-lang/rust-analyzer that referenced this pull request Sep 22, 2023
src/handlers/no_merges.rs Outdated Show resolved Hide resolved
Comment on lines +52 to +55
if config
.exclude_titles
.iter()
.any(|s| event.issue.title.contains(s))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason to not make this case-insensitive? Seems like making it insensitive could make it a little less likely to have false-positives.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously it makes the implementation more complicated, but the real reason is that it reduces specificity.

Capitalization can be used to disambiguate. For example, on rust-clippy, using the capitalized "Rustup" disambiguates between PRs updating from rust and ones just mentioning "rustup"

Since only the first letter in a word is capitalized, generally it only takes a couple more entries to exclude the various combinations if so desired.

RalfJung pushed a commit to RalfJung/rust that referenced this pull request Sep 25, 2023
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Sep 25, 2023
disable no-merges check for now

It leads to false warnings on sync PRs until rust-lang/triagebot#1720 lands.
@pitaj pitaj force-pushed the no-merges_exclude-titles branch 2 times, most recently from 126cac5 to f106af7 Compare September 29, 2023 01:33
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
@pitaj pitaj force-pushed the no-merges_exclude-titles branch from f106af7 to 7fb447a Compare September 29, 2023 01:34
@pitaj pitaj requested a review from ehuss October 3, 2023 01:26
@ehuss ehuss merged commit 75f6785 into rust-lang:master Oct 15, 2023
2 checks passed
Mark-Simulacrum pushed a commit to rust-lang/rust-forge that referenced this pull request Oct 16, 2023
bors added a commit to rust-lang/rust-clippy that referenced this pull request Oct 16, 2023
…ip1995

triagebot no-merges: exclude "Rustup"s, add labels

rust-lang/triagebot#1720

changelog: none
bors added a commit to rust-lang/miri that referenced this pull request Oct 16, 2023
triagebot: re-enable merge commit check

rust-lang/triagebot#1720 has landed.

Also make the keyword "Rustup", which is what we've been already using for such PRs for a while. Just adjust the bot to also put that in the title.
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Oct 21, 2023
triagebot: re-enable merge commit check

rust-lang/triagebot#1720 has landed.

Also make the keyword "Rustup", which is what we've been already using for such PRs for a while. Just adjust the bot to also put that in the title.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants