rustup check#1980
Conversation
|
@chunk0tronic Thank you for your contribution to Rustup. We are indeed interested in seeing this work. It leads into some thoughts I was having regarding how we clobber channel data before we determine if we could update, but I'll worry about that another time -- this sounds like a useful bit of progress. I'm not in a position to do a full review right now since I'm about to go to bed, but I wanted to pop on and say thank you and to say that this is on my radar and I'll try and get to review it soon. If you've not heard from me in a few days please do prod me. You can find me on the Rust lang discord, or on various IRC networks, always as 'Kinnison', or you can poke me via this PR :D |
kinnison
left a comment
There was a problem hiding this comment.
This change is very good. It's close to mergeable with a few small tweaks (See review comments).
In addition though, we prefer to not merge fixup commits if we can avoid them, so please can you ensure you rebase and clean up your commit history. It's nice that you provided a change commit and a test change commit; just fold the fixups and formatting into those.
|
Thanks for your comments! I have rebased on upstream, made your requested updates and squashed the commits so there is still one for implementation and one for tests. |
|
Just noticed my tests commit message needs updating, I'll tweak it and push again. |
|
☔ The latest upstream changes (presumably ec6d1af) made this pull request unmergeable. Please resolve the merge conflicts. |
kinnison
left a comment
There was a problem hiding this comment.
One tiny wart and one query, then 👍 I think.
|
Ok, sliced off the wart and made a simplification based off your query, hope the explanation for how it is now makes sense. The windows test build failed for some reason, is this a real problem? |
|
The Windows failure is a travis bogon. |
|
Yeah! Thanks @kinnison! |
Hi, I was interested in being able to check for updates before running an update, and found that there was already a ticket open for this: #1249. I have had a go at implementing this feature in this PR.
The new command is
rustup checkand the output looks like this:stable-x86_64-unknown-linux-gnu - Up to date : 1.37.0 (eae3437df 2019-08-13)beta-x86_64-unknown-linux-gnu - Up to date : 1.38.0-beta.3 (72bfc3753 2019-08-27)nightly-x86_64-unknown-linux-gnu - Update available : 1.39.0-nightly (fba38ac27 2019-08-31) -> 1.39.0-nightly (dfd43f0fd 2019-09-01)The implementation gets the versions from the current local manifest and the downloaded manifest from the dist server. I added some test cases to the
cli-rustuptests, I had to tweak the construction of the version string in the mock manifests to get this to work properly though.I would appreciate any feedback you can give, some things to highlight:
handle_epipearound thecheck_updatesfunction call and add a newErrorKind, do you think this is necessary?Thanks in advance for your comments.
Preview:
