-
Notifications
You must be signed in to change notification settings - Fork 963
Simplify updates #4404
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
base: master
Are you sure you want to change the base?
Simplify updates #4404
Conversation
I don't think this complexity is worth it? Looking at the changes from when this indirection was introduced does not make it clearer.
.into_iter() | ||
.map(|(p, s)| (PackageUpdate::Toolchain(p), s)) | ||
.collect(); | ||
show_channel_updates(cfg, t)?; |
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.
Sorry, but I'm a bit confused about this... Do you know why are we showing channel updates during self update
in the first place? That might help with understanding the current behavior...
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.
#1605 mentions
This will make sure people who do
rustup update stable
still get a new rustup
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.
@djc Thanks for mentioning that PR!
After another it looks like this change is basically inlining the show_channel_updates()
calls, and the only difference is that in the original version might do other fallible things before printing the channel updates.
Given that, maybe I have misunderstood your original commit message, which gave me the impression of something related to --no-self-update
. How about "Show channel updates even if self update is not permitted", so it's clear that it's related to SelfUpdatePermission
?
Slow followup from