-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 short emoji status to toolstate updates #56758
Conversation
src/tools/publish_toolstate.py
Outdated
@@ -26,14 +26,24 @@ | |||
MAINTAINERS = { | |||
'miri': '@oli-obk @RalfJung @eddyb', | |||
'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk', | |||
'rls': '@nrc @Xanewok', | |||
'rls': '@nrc', |
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.
Is this intentional?
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.
nope, fixed
c25c570
to
ae893bb
Compare
@bors r+ rollup 😈 |
📌 Commit ae893bb has been approved by |
… r=kennytm Add short emoji status to toolstate updates I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly I haven't been able to test it, and the actual emoji are just suggestions.
… r=kennytm Add short emoji status to toolstate updates I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly I haven't been able to test it, and the actual emoji are just suggestions. r? @kennytm cc @rust-lang/infra @rust-lang/devtools
… r=kennytm Add short emoji status to toolstate updates I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly I haven't been able to test it, and the actual emoji are just suggestions. r? @kennytm cc @rust-lang/infra @rust-lang/devtools
… r=kennytm Add short emoji status to toolstate updates I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly I haven't been able to test it, and the actual emoji are just suggestions. r? @kennytm cc @rust-lang/infra @rust-lang/devtools
… r=kennytm Add short emoji status to toolstate updates I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly I haven't been able to test it, and the actual emoji are just suggestions. r? @kennytm cc @rust-lang/infra @rust-lang/devtools
Rollup of 14 pull requests Successful merges: - #56718 (Use libbacktrace pretty-printing) - #56725 (fix rust-lang/rust issue #50583) - #56731 (Add missing urls in ffi module docs) - #56738 (Fix private_no_mangle_fns message grammar) - #56746 (Add test of current behavior (infer free region within closure body)) - #56747 (target: remove Box returned by get_targets) - #56751 (Allow ptr::hash to accept fat pointers) - #56755 (Account for `impl Trait` when suggesting lifetime) - #56758 (Add short emoji status to toolstate updates) - #56760 (Deduplicate unsatisfied trait bounds) - #56769 (Add x86_64-unknown-uefi target) - #56792 (Bootstrap: Add testsuite for compiletest tool) - #56808 (Fixes broken links) - #56809 (Fix docs path to PermissionsExt) Failed merges: r? @ghost
long_message += '🎉 {} on {}: {} → {}.\n' \ | ||
.format(tool, os, old, new) | ||
emoji = "{}🎉".format(EMOJI.get(tool)) | ||
if msg not in emoji_status: |
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.
I may just be missing it but I can't find msg
referenced anywhere else. Should these be emoji
?
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.
This PR broke master. Reverting it.
cc @rust-lang/infra, CI doesn't check if publish_toolstate.py
is valid before merging a PR
@@ -96,6 +111,9 @@ def update_latest( | |||
if not anything_changed: | |||
return '' | |||
|
|||
short_message = "📣 Toolstate changed by {}! ({})" | |||
.format(relevant_pr_number, '/'.join(emoji_status)) |
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.
This is a Python syntax error
Revert merge of #56758 - Manishearth:emoji-status-toolstate #56758 is breaking master. r? @ghost cc @Manishearth @kennytm
I get a lot of these emails and it's good to know which ones I should be paying closer attention to -- i.e. the ones where clippy breaks. This adds a short emoji status report to the first line of the commit message, which shows up in notifications directly
I haven't been able to test it, and the actual emoji are just suggestions.
r? @kennytm
cc @rust-lang/infra @rust-lang/devtools