Skip to content

[5.4][PackageLoading] replace "Next" with "5.4" according to notes left previously #3170

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 21, 2021
Merged

[5.4][PackageLoading] replace "Next" with "5.4" according to notes left previously #3170

merged 2 commits into from
Jan 21, 2021

Conversation

WowbaggersLiquidLunch
Copy link
Contributor

@WowbaggersLiquidLunch WowbaggersLiquidLunch commented Jan 8, 2021 •
edited
Loading

#2937 used "Next" and ToolsVersion.vNext because at that point it was unconfirmed what the next version of Swift was. Now that the 5.4 branch is cut, I corrected all the references to Swift Next to Swift 5.4, according to rules I wrote in the comments in the previous PR.

EDIT: By "all the references to Swift Next", I mean all those introduced in #2937. There are other "Next" and .vNext not changed by this PR.

@WowbaggersLiquidLunch WowbaggersLiquidLunch changed the title [5.4] [PackageLoading] replace "Next" with "5.4" according to notes left previously [5.4][PackageLoading] replace "Next" with "5.4" according to notes left previously Jan 8, 2021
@tomerd tomerd added the 5.4 label Jan 8, 2021
abertelrud
abertelrud approved these changes Jan 9, 2021
View reviewed changes
Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

Thanks!

@abertelrud
Copy link
Contributor

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Would you mind also opening a separate PR for main? (referencing this PR somewhere in that PR). Thanks!

@WowbaggersLiquidLunch
Copy link
Contributor Author

Would you mind also opening a separate PR for main? (referencing this PR somewhere in that PR). Thanks!

Sure. I was thinking of cherry-picking this the commit to main once this PR is merged. But I can open a new PR as well.

@WowbaggersLiquidLunch WowbaggersLiquidLunch mentioned this pull request Jan 9, 2021
Merged
@abertelrud
Copy link
Contributor

Looks like a full test is required for 5.4...

@abertelrud
Copy link
Contributor

@swift-ci please test

@tomerd
Copy link
Contributor

tomerd commented Jan 9, 2021

@swift-ci please smoke test

@WowbaggersLiquidLunch
Copy link
Contributor Author

Sorry I found a single space that I forgot to remove in line 444 of ToolsVersionLoader.swift. I removed it, then squashed the commit and force-pushed the branch. The PR will need to be tested again.

@WowbaggersLiquidLunch
[5.4] [PackageLoading] replace Next with 5.4 according to notes/instr…
e03ffa3 
…uctions left previously

Also added `ToolsVersion.v5_4`.
@WowbaggersLiquidLunch
Copy link
Contributor Author

Just removed a few more spaces from around the same place. This is my last modification to this PR. No more changes unless requested. Sorry for disrupting the tests!

@abertelrud
Copy link
Contributor

@swift-ci please test

@WowbaggersLiquidLunch
[NFC] clarify comments in the tests
3d383d5 
Empty lines before the Swift tools version specification comment are not supported for Swift < 5.4.
@abertelrud
Copy link
Contributor

@swift-ci please test

@abertelrud
Copy link
Contributor

I have also tested locally with a patch of this, and all the tests pass. I think this is good to be nominated.

@abertelrud
Copy link
Contributor

java.io.EOFException. Trying again.

@abertelrud
Copy link
Contributor

@swift-ci please test macOS

@abertelrud abertelrud mentioned this pull request Jan 14, 2021
Merged
@abertelrud
Copy link
Contributor

Looking into the error here.

Note: PR #3194 is the most critical one to get in; this is important clean-up in addition to that, but not as critical in the short term.

@abertelrud
Copy link
Contributor

The fixes have landed, so this should succeed now. Builds are rerunning.

@abertelrud
Copy link
Contributor

This one should now be ready to merge. @tomerd I think there were just a couple of comment tweaks in the tests since you approved it. All good to merge to 5.4 branch?

@tomerd
Copy link
Contributor

tomerd commented Jan 20, 2021

👍

@abertelrud abertelrud merged commit b7b52cc into swiftlang:release/5.4 Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abertelrud abertelrud abertelrud approved these changes

@federicobucchi federicobucchi federicobucchi approved these changes

@tomerd tomerd tomerd approved these changes

@aciidgh aciidgh Awaiting requested review from aciidgh

@friedbunny friedbunny Awaiting requested review from friedbunny friedbunny is a code owner

@neonichu neonichu Awaiting requested review from neonichu neonichu is a code owner

@airspeedswift airspeedswift Awaiting requested review from airspeedswift

Assignees

@abertelrud abertelrud

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@WowbaggersLiquidLunch @abertelrud @tomerd @federicobucchi