Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager via the forum messaging feature. When contacting the review manager directly, please keep the proposal link at the top of the message.
Try it out
If you'd like to try this proposal out, you can download a toolchain supporting it for Linux, Windows, or macOS using a recent main development snapshot from Swift.org - Download Swift and run the swift package migrate command.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
perhaps more of a question than a proposal evaluation, but i saw in the pitch some discussion regarding the feasibility of using the new migration mode in the compiler to power 3rd party tooling to apply the automated migration changes. however, it's not clear to me how straightforward this would be to do given the current proposal.
from looking through the source a bit, it seems like non-SPM build systems would be able to easily invoke the new migration compiler commands, but the subsequent step of processing serialized diagnostics to extract, filter, and apply any relevant fix-its would perhaps have to be re-implemented separately from the implementation in SPM.
having the ability to apply fix-its in an automated manner seems like it could be a generically useful ability, independent of managing feature migrations. swiftc even has a (seemingly-now-defunct) flag '-fixit-all' that suggests it at one time did something like this.
i'm curious to hear if the authors have any thoughts about the possibility of exposing some of the more generic internal components built for the migration feature to support other use cases (or the same use case in other build environments).
i'm curious to hear if the authors have any thoughts about the possibility of exposing some of the more generic internal components built for the migration feature to support other use cases (or the same use case in other build environments).
The implementation currently lives in SwiftPM but it's not necessary has to be located there (even the command itself), we are looking into moving most of the functionality including SwiftFixIt and package manifest refactorings over to SwiftSyntax, which should make it much simpler to adopt in other tooling.
We have both --targets (plural comma separated) and --to-feature (singular allowing multiple options) in the interface here. Either is fine, but might make sense to have them be consistent (either plural or singular but not both)? So eg.
swift package migrate --target Foo --target Bar --to-feature Baz --to-feature Boop
or
swift package migrate --targets Foo,Bar --to-features Baz,Boop
Could check what we do for other commands, though I suspect we may be a little inconsistent there too
As for the naming swift migrate versus swift package migrate, we'll need to revisit our naming strategy holistically at some point. There are too many sub commands for package making things hard for users to find and discover. I understand that it's easier to add new tools there that than add another top level tool to the toolchain, but that needs to be made easier as well.
Given we have a lot of work to clean things up, I am OK for now with the choice of the authors.
Maybe I'm reading that wrong, but it sounds like the compiler interface emits the fix-its, but doesn't actually perform the migration like swift package migrate does.
error: feature 'NonisolatedNonsendingByDefault' will cause nonisolated async instance method to run on the caller's actor; use '@concurrent' to preserve behavior
One piece of feedback from using the NonisolatedNonsendingByDefault:migrate flag is that it's pertinent to know which actor the "caller's actor" is, in order to decide if @concurrent should be applied, mostly to understand if the MainActor is what might be blocked.
