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

Change the Gutenberg integration to use XCFramework #20717

Closed
wants to merge 29 commits into from

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented May 21, 2023

This one was a long time coming... kept having to tweak the setup 😰

This PRs adds all the setup necessary to fetch and use Gutenberg as an XCFramework via commit or tag.

The setup is relatively straightforward. If Gutenberg/version.rb specifies a commit, the code that declares the pod 'Gutenberg' dependency will try and fetch the podspec for that commit from our CDN.

You can see the setup in action in #20792 and #20845. In #20845 in particular, notice how everything is green apart from a UI test which is 1) unrelated to the editor (insights) and 2) most likely failing due to async behavior.

image

Notice that this PR doesn't switch Gutenberg to a commit or tag. Assuming this PR will be merged before the next minor version of Gutenberg ships (meaning 1.98.0), the switch will happen when the Gutenberg folks update to that new version.

Testing

You can try replicating the changes from #20792 locally, then run bundle exec pod install. Verify then that the editor works as expected. @fluiddot @geriux @SiobhyB @dcalhoun I'd appreciate your input as well as knowing if I wired up things correctly. Thanks!

Regression Notes

  1. Potential unintended areas of impact – N.A.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

PR submission checklist:

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

UI changes testing checklist: Not a UI PR.

@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch from d8bd8f1 to cc51e8b Compare May 22, 2023 04:34
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch 2 times, most recently from a2500f3 to cb4b3e0 Compare May 23, 2023 04:26
@mokagio mokagio changed the title Changed Gutenberg integration to use XCFramework Change the Gutenberg integration to use XCFramework May 24, 2023
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch from cb4b3e0 to 25884d2 Compare May 25, 2023 03:56
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch from 25884d2 to 6f74940 Compare May 25, 2023 12:12
@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 25, 2023

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 Numberpr20717-d6dd34e
Version22.7
Bundle IDcom.jetpack.alpha
Commitd6dd34e
App Center Buildjetpack-installable-builds #5256
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 May 25, 2023

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 Numberpr20717-d6dd34e
Version22.7
Bundle IDorg.wordpress.alpha
Commitd6dd34e
App Center BuildWPiOS - One-Offs #6230
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch 2 times, most recently from 0ff26e5 to 5b7a7b7 Compare June 2, 2023 03:17
Gutenberg/Gutenberg.podspec Outdated Show resolved Hide resolved
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch 3 times, most recently from ec6fe5f to b2ecf7b Compare June 5, 2023 11:30
@mokagio mokagio marked this pull request as ready for review June 5, 2023 11:42
@mokagio mokagio marked this pull request as draft June 5, 2023 11:42
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch 5 times, most recently from 056d7c5 to 1df239d Compare June 13, 2023 03:58
IMAGE_ID: xcode-14.2
IMAGE_ID: xcode-14.3.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admittedly, this was unrelated with the Gutenberg setup.

I did run into some Xcode version compatibility issue during the long development of this integration, but I think it should no longer be a problem.

Having said that, I think we might as well use the latest toolchain, no?

@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch 2 times, most recently from af3b326 to 081ea98 Compare June 13, 2023 08:48
@mokagio mokagio requested a review from crazytonyli June 13, 2023 09:25
@mokagio mokagio added the Tooling Build, Release, and Validation Tools label Jun 13, 2023
@mokagio mokagio added this to the 22.7 milestone Jun 13, 2023
mokagio and others added 2 commits July 3, 2023 15:51
Co-authored-by: Tony Li <tony.li@automattic.com>
Co-authored-by: Tony Li <tony.li@automattic.com>
@mokagio mokagio force-pushed the mokagio/gutenberg-xcframework-setup branch from 0f8cc00 to c2c46cc Compare July 3, 2023 05:52
#
# [!] An error occurred while processing the post-install hook of the Podfile.
#
# No such file or directory @ rb_sysopen - /Users/gio/Developer/a8c/wpios/Users/gio/Developer/a8c/wpios/Gutenberg/../../gutenberg-mobile/gutenberg/node_modules/react-native/package.json
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 Line is too long. [189/180]

@fluiddot
Copy link
Contributor

fluiddot commented Jul 5, 2023

Heads up that I updated the PR related to the RN upgrade 0.71.11 with these changes. I'm happy to share that the installable builds were created 🎊.

There are only two minor changes on that PR compared to this one:

@fluiddot
Copy link
Contributor

fluiddot commented Jul 6, 2023

@mokagio Heads up that I created this PR to fix the current failures we have in CI jobs. Let me know if you could take a look, thanks 🙇 !

@mokagio
Copy link
Contributor Author

mokagio commented Jul 7, 2023

Thanks @fluiddot!

I received your message but I might run out of time to look into it today. I have a lot of PRs to review for the upcoming 22.8 code freeze.

@mokagio mokagio modified the milestones: 22.8, 22.9 Jul 7, 2023
@fluiddot
Copy link
Contributor

fluiddot commented Jul 7, 2023

Thanks @fluiddot!

I received your message but I might run out of time to look into it today. I have a lot of PRs to review for the upcoming 22.8 code freeze.

No worries at all @mokagio. We can revisit this next week if you have more availability. Thanks!

UPDATE: I've just realized you will be away next week, so we can postpone this to the week after the next one.

@fluiddot
Copy link
Contributor

@mokagio Heads up that I've updated the PR with trunk to get recent changes. From my side the PR is ready but I understand that we'd need first to land:

Let me know if I'm missing anything to set the XCFramework integration ready.

The UI tests are still failing even when using the latest changes introduced in the editor. We should keep investigating how to address them.

Regarding the merge, I'd like to note that since we need changes from the React Native upgrade, we should wait until those changes are merged. Alternatively, I'm updating #20956 with all changes needed for the RN upgrade. Maybe it's easier to merge them through that PR.

@fluiddot
Copy link
Contributor

Closing in favor of #20956, as these changes need to be merged along with the React Native upgrade 0.71.11.

Heads up that I'll re-assign current reviewers to that PR to validate the XCFramework integration. Thanks for your help 🙇 !

@fluiddot fluiddot closed this Jul 21, 2023
auto-merge was automatically disabled July 21, 2023 08:58

Pull request was closed

@mokagio mokagio deleted the mokagio/gutenberg-xcframework-setup branch July 24, 2023 06:32
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.

5 participants