Skip to content

Conversation

patrickfreed
Copy link
Contributor

Closes #71

Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

Looks good. One minor question.


print("updating \(oldToolchain) -> \(newToolchain)")
try await Install.execute(version: newToolchain)
try await Install.execute(version: newToolchain, use: false)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know update hasn't been merged yet but shouldn't swiftly update also default to using the newly updated toolchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just in here to have the existing update code compile, but it'll be changed in #67. The planned logic for this is if the toolchain being updated is the one that is in use, then the newly installed one will be used, otherwise no.

@patrickfreed
Copy link
Contributor Author

Going to wait to merge this until #67 goes in so that I can reuse the download mocking for the test I added here. The RHEL 9 failure is due to a missing download on that platform, but we don't actually need to download anything for this test.

@patrickfreed patrickfreed merged commit d698703 into swiftlang:main Nov 3, 2023
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.

Provide option to use the newly installed toolchain
2 participants