-
Notifications
You must be signed in to change notification settings - Fork 84
Install Toolchain #1780
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
Install Toolchain #1780
Conversation
845d30a to
3b13b09
Compare
matthewbastien
left a comment
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.
Looking really good so far! Here are some things I noticed while testing this out locally:
- The toolchain selection dialog is getting really big with this change. I think it makes more sense to make the Swiftly install a separate VS Code command and add it as an action to the toolchain selection dialog. Toolchain selection is more about switching between already installed toolchains anyway.
- Toolchains should be ordered most recent first rather than earliest. E.g. 6.1.1 should be at the top.
5cd66b3 to
76f3df7
Compare
award999
left a comment
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.
Looking good, some smaller things
80b67fe to
a1515c6
Compare
a1515c6 to
9ef0ecf
Compare
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 awesome! Thank you!
I left one comment about the snapshot installation not working correctly on my machine, but I think I'll just raise a new issue for it to address later.
| return; | ||
| } | ||
|
|
||
| const availableToolchains = await Swiftly.listAvailable(ctx.logger); |
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 can be addressed in a future PR, but on my machine swiftly list-available does not show any snapshots by default. I have to be more specific and explicitly ask for 6.2-snapshot: swiftly list-available 6.2-snapshot.
This command always says "you have installed all available snapshots" even though I could update to a newer 6.2-snapshot.
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.
#1807 Addressed

Feature Issue #1778
Description
Issue: 1778
Tasks