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

Update RNTAztecView.podspec dependency #54453

Merged
merged 6 commits into from
Sep 15, 2023
Merged

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Sep 14, 2023

What?

Updates RNTAztecView to use the latest Aztec iOS build wordpress-mobile/AztecEditor-iOS#1376

Why?

To address a bug in iOS 17.

How?

By using ~> 1.19. While this does explicitly use 1.19.9, it will let every client update to it as long as its upstream dependencies are compatible.

Testing Instructions

I'm not sure about this. The Xcode project in the module doesn't build in isolation because it depends on React files that are only available when it's imported. 🤔

Maybe I should create a PR in gutenberg-mobile that uses this Gutenberg version and ensure it works. Update: I created wordpress-mobile/gutenberg-mobile#6201 to test this change works in gutenberg-mobile, when build on top of wordpress-mobile/gutenberg-mobile#6200 which makes this repo the source of truth for RNTAztecView.podspec.

Testing Instructions for Keyboard

N.A.

Screenshots or screencast

N.A.

@geriux geriux added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) Mobile App - Automation Label used to initiate Mobile App PR Automation [Type] Task Issues or PRs that have been broken down into an individual action to take labels Sep 14, 2023
@geriux geriux removed the Mobile App - Automation Label used to initiate Mobile App PR Automation label Sep 14, 2023
@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Flaky tests detected in 1f05df4.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6197739745
📝 Reported issues:

@mokagio mokagio enabled auto-merge (squash) September 14, 2023 06:41
@fluiddot fluiddot self-requested a review September 14, 2023 08:27
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we still need these files 🤔. I presume that if they were pointing to an older version than the one referenced in RNTAztecView.podspec, probably we could deprecate them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree in that we should drop using Carthage. As you point out, the version was out of sync so either it's not used, something is misconfigured, or we've been lucky.

However, calling carthage it's part of the package.json scripts. I think the reason it's there is to manage the Aztec - Editor dependency without CocoaPods. I think getting rid of it is the work for a dedicated PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

However, calling carthage it's part of the package.json scripts. I think the reason it's there is to manage the Aztec - Editor dependency without CocoaPods. I think getting rid of it is the work for a dedicated PR.

That's interesting. As you pointed out, carthage was probably added as an alternative to CocoaPods. I'm curious if it's actually being used in the iOS Gutenberg demo app. My gut feeling is that it's legacy and we could remove it, but let's tackle this in a separate PR 👍 .

@fluiddot
Copy link
Contributor

@mokagio heads up that I updated the Podfile.lock in 1f05df4 by running the command npm run native preios.

@fluiddot fluiddot added Mobile App - Automation Label used to initiate Mobile App PR Automation [Type] Build Tooling Issues or PRs related to build tooling and removed [Type] Task Issues or PRs that have been broken down into an individual action to take labels Sep 15, 2023
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

@mokagio mokagio merged commit 069a8f8 into trunk Sep 15, 2023
52 checks passed
@mokagio mokagio deleted the mokagio/update-aztec-ios branch September 15, 2023 14:51
@github-actions github-actions bot added this to the Gutenberg 16.7 milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - Automation Label used to initiate Mobile App PR Automation Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants