Skip to content

Cleanup Keystone and WordPressData Objective-C import #24628

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

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 26, 2025

In my effort to get Keystone to build for testing (Keystone scheme > Shift Cmd U) I'm running into weird build failures.

One of the things I've done to try and wrangle them is to tidy up the Objective-C imports for Keystone and WordPressData.

This PR:

  • Updates all Objective-C headers to prefer forward declaration whenever possible for WordPressData, to keep the build lean
  • Use modular header for WordPressData, @import WordPressData
  • Remove the unused #ifdef KEYSTONE. Notice that the custom build setting had removed weeks ago via 216b4c6

@dangermattic
Copy link
Collaborator

dangermattic commented Jun 26, 2025

1 Warning
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

@mokagio mokagio added this to the 26.3 milestone Jun 26, 2025
@mokagio mokagio added the Tooling Build, Release, and Validation Tools label Jun 26, 2025
@mokagio mokagio self-assigned this Jun 26, 2025
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 26, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number28044
VersionPR #24628
Bundle IDorg.wordpress.alpha
Commit5ad3fbd
Installation URL3ojhhtvipn5co
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 26, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number28044
VersionPR #24628
Bundle IDcom.jetpack.alpha
Commit5ad3fbd
Installation URL7mq4nv2l93i1g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Base automatically changed from mokagio/add-some-imports-for-keystone-20250626 to trunk June 27, 2025 00:54
mokagio added 5 commits June 27, 2025 11:19
Just trying to understand why Keystone fails to build for testing.

Using forward declarations keeps the headers and the compilation leaner.
Clearly the issues documented in some of the usages were temporary due
to the transition.
@mokagio mokagio force-pushed the mokagio/cleanup-20250626-objc-imports branch from d5a3720 to 5ad3fbd Compare June 27, 2025 01:22
@mokagio mokagio requested review from kean and crazytonyli June 27, 2025 01:24
Copy link

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