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

Bug 1870 - UI edition #1885

Merged
merged 2 commits into from
Jun 3, 2019
Merged

Bug 1870 - UI edition #1885

merged 2 commits into from
Jun 3, 2019

Conversation

rbtcollins
Copy link
Contributor

Bug 1870's fix didn't show users what was going on and could stall for up to 30 seconds while waiting. This will show info: messages when that happens; perhaps a lot of them, but better on balance I think than absolutely nothing. Further refinement is of course possible but I think this is the minimum needed to make more releases sensible to do.

The sender can be parameterised with a type parameter allowing
write-once handlers that can handle any Notification type within
their enum hierarchy as long as From is present, unlike the receiver
(because the receiver is passed as a parameter to the function that
will be doing the callback.)

Less adapters, less closures, less headaches when introducing UI
interactions to currently UI-interactionless codepaths.
Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

Fundamentally looks good.

There's a formatting bogon and I think one missed spot where you need to qualify the _ argument in a closure, but otherwise I think this is likely good to go.

My only concern is how much extra spew it produces. Is it possible for you to include a comment on the PR showing the increased verbosity, or are you stuck not able to trigger the issue yourself?

Once you've fixed the CI bogons, I'll approve.

When we spin retrying a rename, show a UI notification.

Rather raw but much better than silently doing nothing for an extended
period.
@kinnison kinnison merged commit 2e97e32 into rust-lang:master Jun 3, 2019
@rbtcollins
Copy link
Contributor Author

rbtcollins commented Jun 3, 2019 via email

@rbtcollins rbtcollins deleted the bug-1870 branch March 13, 2021 21:22
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