-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add swift version file to record the Swift toolchain to use #8411
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
6.0.3 | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -88,7 +88,13 @@ $> swift --version | |||||||||||||||||
Apple Swift version 5.3 | ||||||||||||||||||
``` | ||||||||||||||||||
|
||||||||||||||||||
Note: Alternatively use tools like [swiftenv](https://github.com/kylef/swiftenv) that help manage toolchains versions. | ||||||||||||||||||
Alternatively, there are tools like [swiftly](https://github.com/swiftlang/swiftly) that can install and manage toolchains automatically. This repository has a file called `.swift-version` that will keep swiftly at the current recommended version of the toolchain for best results. The `swiftly install` command ensures that SwiftPM's in-use toolchain is installed on your system and ready for you to do your development work with the usual swift commands. | ||||||||||||||||||
|
||||||||||||||||||
``` bash | ||||||||||||||||||
swiftly install | ||||||||||||||||||
swift build | ||||||||||||||||||
swift test | ||||||||||||||||||
``` | ||||||||||||||||||
|
||||||||||||||||||
## Local Development | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -322,14 +328,15 @@ Note there are several Linux and Swift versions options to choose from, e.g.: | |||||||||||||||||
2. Clone a working copy of your fork | ||||||||||||||||||
3. Create a new branch | ||||||||||||||||||
4. Make your code changes | ||||||||||||||||||
Comment on lines
328
to
330
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's update punctuation in each point for consistency.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above. |
||||||||||||||||||
5. Try to keep your changes (when possible) below 200 lines of code. | ||||||||||||||||||
6. We use [SwiftFormat](https://www.github.com/nicklockwood/SwiftFormat) to enforce code style. Please install and run SwiftFormat before submitting your PR, ideally isolating formatting changes only to code changed for the original goal of the PR. This will keep the PR diff smaller. | ||||||||||||||||||
7. Commit (include the Radar link or GitHub issue id in the commit message if possible and a description your changes). Try to have only 1 commit in your PR (but, of course, if you add changes that can be helpful to be kept aside from the previous commit, make a new commit for them). | ||||||||||||||||||
8. Push the commit / branch to your fork | ||||||||||||||||||
9. Make a PR from your fork / branch to `apple: main` | ||||||||||||||||||
10. While creating your PR, make sure to follow the PR Template providing information about the motivation and highlighting the changes. | ||||||||||||||||||
11. Reviewers are going to be automatically added to your PR | ||||||||||||||||||
12. Pull requests will be merged by the maintainers after it passes CI testing and receives approval from one or more reviewers. Merge timing may be impacted by release schedule considerations. | ||||||||||||||||||
5. If a particular version of the Swift toolchain is needed then update the `.swift-version` file to that version (or use `swiftly use` to update it). | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should encourage contributors to bump the version requirement. It would misalign with the version used on CI then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this would be a great discussion point in any PR that attempts to use features of a newer toolchain and pinpoint exactly what code changes require the newer version. Updating the swift version file reveals that, and someday the hope is that the CI would attempt to use that toolchain to verify the PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my opinion, the CIs pipelines should use the version of the toolchain specified in the We should work on getting our current pipelines updated to install said version if a |
||||||||||||||||||
6. Try to keep your changes (when possible) below 200 lines of code. | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm hoping to keep this PR scoped to just the changes needed for tracking swift toolchain version. The only reason this PR changes this line is to renumber the ordered list to insert. I would have preferred to use the GitHub markdown auto-numbering feature. Further editing of the contribution guide can be tracked in another PR. |
||||||||||||||||||
7. We use [SwiftFormat](https://www.github.com/nicklockwood/SwiftFormat) to enforce code style. Please install and run SwiftFormat before submitting your PR, ideally isolating formatting changes only to code changed for the original goal of the PR. This will keep the PR diff smaller. | ||||||||||||||||||
8. Commit (include the Radar link or GitHub issue id in the commit message if possible and a description your changes). Try to have only 1 commit in your PR (but, of course, if you add changes that can be helpful to be kept aside from the previous commit, make a new commit for them). | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The last sentence here is not relevant, people can have as many commits (including merge commits) in their PRs as they please since we squash before merging anyway.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above. Can we focus on the tracking of the swift toolchain version in this PR? I don't mind reviewing further refinements to the contribution guide separately. |
||||||||||||||||||
9. Push the commit / branch to your fork | ||||||||||||||||||
10. Make a PR from your fork / branch to `apple: main` | ||||||||||||||||||
11. While creating your PR, make sure to follow the PR Template providing information about the motivation and highlighting the changes. | ||||||||||||||||||
12. Reviewers are going to be automatically added to your PR | ||||||||||||||||||
Comment on lines
+335
to
+338
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: punctuation consistency
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comments. |
||||||||||||||||||
13. Pull requests will be merged by the maintainers after it passes CI testing and receives approval from one or more reviewers. Merge timing may be impacted by release schedule considerations. | ||||||||||||||||||
|
||||||||||||||||||
By submitting a pull request, you represent that you have the right to license | ||||||||||||||||||
your contribution to Apple and the community, and agree by submitting the patch | ||||||||||||||||||
|
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 file is missing a trailing newline. Additionally, could it have a comment on the first or second line pointing to relevant documentation for discoverability?
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 file should be dead simple, able to be
cat
'ed as part of scripts/commands to easily get the desired toolchain without any further processing.