Skip to content
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

WordPressKit as a binary framework #23418

Merged
merged 5 commits into from
Jul 16, 2024
Merged

WordPressKit as a binary framework #23418

merged 5 commits into from
Jul 16, 2024

Conversation

kean
Copy link
Contributor

@kean kean commented Jul 12, 2024

Changes

  • Install WordPressKit using SwiftPM and remove the code from the repo
  • Remove wpxmlrpc dependency from the app
  • Remove NSObject-SafeExpectations dependency from WordPressAuthentificator

WordPressKit can now be used as a dependency for any of the internal packages we introduce. WordPressAuthentificator now also has a path to becoming a SwiftPM module.

It also slightly reduces the build time: 103 sec → 94 sec (-9 sec or nearly 9%). It is a bit hard to measure accurately, so take it with a grain of salt.

Related PR: wordpress-mobile/WordPressKit-iOS#816

Remainig steps:

The app already runs, but just to be safe, we need to ensure that the Objective-C dependencies that are currently linked statically in WordPressKit do not create any ambiguity in the Objective-C runtime. I achieved that with WordPressShared by inlining its sources in WordPressKit and prefixing the Objective-C methods with wpkit_.

To test:

Regression Notes

  1. Potential unintended areas of impact

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@kean kean requested a review from a team as a code owner July 12, 2024 14:53
@kean kean changed the base branch from trunk to task/modularization-p2 July 12, 2024 14:53
@dangermattic
Copy link
Collaborator

dangermattic commented Jul 12, 2024

2 Warnings
⚠️ Modules/Package.swift was changed without updating its corresponding Package.resolved. Please resolve the Swift packages in Xcode.
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 12, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23418-69e3c05
Version25.1
Bundle IDorg.wordpress.alpha
Commit69e3c05
App Center BuildWPiOS - One-Offs #10294
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 12, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23418-69e3c05
Version25.1
Bundle IDcom.jetpack.alpha
Commit69e3c05
App Center Buildjetpack-installable-builds #9341
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean kean marked this pull request as draft July 12, 2024 15:10
Base automatically changed from task/modularization-p2 to trunk July 12, 2024 23:49
@kean kean force-pushed the task/remove-wpkit-package branch from 55a5732 to 61dac1a Compare July 13, 2024 01:15
@kean kean changed the title [Experimental] WordPressKit binary framework WordPressKit as a binary framework Jul 13, 2024
@kean kean force-pushed the task/remove-wpkit-package branch from 61dac1a to 69e3c05 Compare July 13, 2024 01:28
@kean kean marked this pull request as ready for review July 15, 2024 13:42
@kean kean requested a review from jkmassel July 15, 2024 13:42
@kean kean added the Tooling Build, Release, and Validation Tools label Jul 15, 2024
@kean kean added this to the Pending milestone Jul 15, 2024
@kean kean added this pull request to the merge queue Jul 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 16, 2024
@kean kean added this pull request to the merge queue Jul 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 16, 2024
@kean kean added this pull request to the merge queue Jul 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jul 16, 2024
@kean kean merged commit 5316a57 into trunk Jul 16, 2024
25 of 26 checks passed
@kean kean deleted the task/remove-wpkit-package branch July 16, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tooling Build, Release, and Validation Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants