Simplify Experiments into a Swift package #15622
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated by 🚫 Danger |
mokagio
force-pushed
the
ainfra-471-move-experiments-to-modules
branch
from
9423902 to
acd51eb
Compare
|
|
mokagio
force-pushed
the
ainfra-471-move-experiments-to-modules
branch
from
acd51eb to
eae2e4c
Compare
mokagio
force-pushed
the
ainfra-471-move-experiments-to-modules
branch
from
eae2e4c to
9cbaeb7
Compare
mokagio
added
the
category: tooling
mokagio
added a commit
that referenced
this pull request
Jun 2, 2025
When moving from frameworks to Swift packages, see for example #15622 and #15651, we lose the framework headers which had statements like `#import <Foundation/Foundation.h>`. This means that basic frameworks such as Foundation and UIKit are no longer imported as part of consumers frameworks such as Yosemite. To simplify the code review of the upcoming PRs migrating those frameworks to Swift packages in `Modules/`, this PR preemptively adds the required imports in the files that need them. This was done by running `git reset trunk` on the workbench branch `temp-branch-for-swiftpm-migration` and only committing the `import` additions in this branch.
1 task
mokagio
added a commit
that referenced
this pull request
Jun 2, 2025
When moving from frameworks to Swift packages, see for example #15622 and #15651, we lose the framework headers which had statements like `#import <Foundation/Foundation.h>`. This means that basic frameworks such as Foundation and UIKit are no longer imported as part of consumers frameworks such as Yosemite. To simplify the code review of the upcoming PRs migrating those frameworks to Swift packages in `Modules/`, this PR preemptively adds the required imports in the files that need them. This was done by running `git reset trunk` on the workbench branch `temp-branch-for-swiftpm-migration` and only committing the `import` additions in this branch.
mokagio
force-pushed
the
ainfra-471-move-experiments-to-modules
branch
from
9cbaeb7 to
1d33e99
Compare
mokagio
commented
Jun 3, 2025
| @@ -1,6 +1,7 @@ | |||
| import Foundation | |||
| import KeychainAccess | |||
| import Networking | |||
| import UserNotifications | |||
There was a problem hiding this comment.
Explanation for why this and the Foundation import below are here can be found in #15698
twstokes
reviewed
Jun 4, 2025
There was a problem hiding this comment.
Actually on second thought @mokagio - when trying to run WCiOS in a Simulator I'm seeing:
dyld[36556]: Symbol not found: _$sSo7UIColorC13WooFoundationE15withColorStudio_5shadeAB0bC4Core0eF0V_AF0eF5ShadeOtFZ
Referenced from: <A6BCB995-881E-31AF-92A5-6146F3FFD8EC> /Users/tanner/Library/Developer/CoreSimulator/Devices/7586F18C-8D66-495B-BFB8-6346FCF526B1/data/Containers/Bundle/Application/EF3681BC-7340-4715-B3A4-A2E30D1DF40C/WooCommerce.app/WooCommerce.debug.dylib
Expected in: <5F3EA820-850E-3F18-91A5-97361DE9CEDD> /Users/tanner/Library/Developer/Xcode/DerivedData/WooCommerce-dutzhurnuvsqusgjnyibtbncmmou/Build/Products/Debug-iphonesimulator/WooFoundation.framework/WooFoundation
I'll double-check that this isn't something in my local environment.
Edit: After cleaning my local environment and rebuilding I'm able to run the app without this crash.
twstokes
approved these changes
Jun 4, 2025
|
Thanks @twstokes ! Regarding the issue that resolved itself after cleaning, I'm afraid we can expect a few of those as we transition. A positive side effect of having to bulk move all the remaining frameworks because of how Fakes is used in the tests and links to all of them, #15678, is that we don't have many PRs left till the migration is done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Similar too #15651 , this PR converts the Experiments framework into a Swift package in
Modules/.See also https://linear.app/a8c/issue/AINFRA-471
Unfortunately, this is the last individual framework migration we can perform. The rest will need to be moved in bulk because the tests all depend on
Fakes, which in turn depends on the rest of the frameworks.Steps to reproduce & Testing information
See green CI + run this branch on device.
Screenshots
RELEASE-NOTES.txtif necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: