-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Refactor: Add WordPressMedia #23890
Refactor: Add WordPressMedia #23890
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
Do you think it's worthwhile enabling strict concurrency checks on the new targets? If so, we can wait for #23838 to be merged. |
import Foundation | ||
|
||
extension Bundle { | ||
public static var test: Bundle { Bundle.module } |
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.
test
seems too generic. What do you think of naming it wordPressTesting
?
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.
Sure. New modules like this should start with Swift 6 enabled. I approved the PR you referenced. |
Thanks! The PR has been merged. |
8dccafd
to
ea24202
Compare
I rebased it (with no changes) and added Swift 6 support in ea24202. The changes should be pretty straightforward. I added |
I'm going to merge this later after 24.6 is properly cut-off. |
ea24202
to
9ee042e
Compare
This PR enables a bit of modularization by extracting some obvious stuff:
ImageDownloader
. The framework name isWordPressMedia
because the idea is to put everything media-related there, including exporters in the future.The main advantage of this change is that you can now compile and run tests for this new target nearly instantly, so it will encourage us to add more tests and make it easier to maintain existing ones.
Changes:
WordPressMedia
targetImageDownloader
and the types it depend on to the new targetImageDownloader
tests to use the new Swift Testing frameworkWordPressTesting
target to share test code and resources across test targetsRegression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: