-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Only create a relnotes tracking issue for finished FCPs if they have disposition merge #1904
Only create a relnotes tracking issue for finished FCPs if they have disposition merge #1904
Conversation
@@ -27,6 +27,8 @@ struct RelnotesState { | |||
relnotes_issue: Option<u64>, | |||
} | |||
|
|||
const TITLE_PREFIX: &str = "Tracking issue for release notes"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by refactoring to ensure that the two usage sites don't get out of sync.
src/handlers/relnotes.rs
Outdated
&& e.issue | ||
.labels | ||
.iter() | ||
.any(|label| label.name == "disposition-merge") | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice this shoudn't be prone to race conditions because disposition-*
gets applied during proposed-final-comment-period
already.
src/handlers/relnotes.rs
Outdated
&& e.issue | ||
.labels | ||
.iter() | ||
.any(|label| label.name == "disposition-merge") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add parentheses or perhaps extract let is_fcp_merge = ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied.
…disposition merge
5642640
to
e72f53c
Compare
Addresses rust-lang/rust#137075 (comment)
r? @Mark-Simulacrum || (release ∩ infra)