Skip to content

Convert ReaderPost mapping to Swift#25301

Open
kean wants to merge 6 commits intotrunkfrom
task/convert-remote-post-mapper
Open

Convert ReaderPost mapping to Swift#25301
kean wants to merge 6 commits intotrunkfrom
task/convert-remote-post-mapper

Conversation

@kean
Copy link
Contributor

@kean kean commented Feb 25, 2026

I used TDD to reduce the risk of regression and performed manual tests.

@kean kean added the Tech Debt label Feb 25, 2026
@kean kean added this to the Someday milestone Feb 25, 2026
@dangermattic
Copy link
Collaborator

dangermattic commented Feb 25, 2026

2 Warnings
⚠️ Modules/Package.swift was changed without updating its corresponding Package.resolved.

If the change includes adding, removing, or editing a dependency please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running swift package resolve).

If the change to the Package.swift did not modify dependencies, ignoring this warning should be safe, but we recommend double checking and running the package resolution just in case.
.

⚠️ 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 Feb 25, 2026

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 Number31223
VersionPR #25301
Bundle IDorg.wordpress.alpha
Commit985fa3a
Installation URL1i5ijmrdsphl0
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 Feb 25, 2026

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 Number31223
VersionPR #25301
Bundle IDcom.jetpack.alpha
Commit985fa3a
Installation URL10pq56igr4790
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@kean kean requested review from crazytonyli and jkmassel February 25, 2026 14:41
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 25, 2026

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

@kean kean enabled auto-merge February 25, 2026 17:27

private extension ReaderPost {
func updateCrossPostMeta(from remotePost: RemoteReaderPost, in context: NSManagedObjectContext) {
guard let remoteMeta = remotePost.value(forKey: "crossPostMeta") as? RemoteReaderCrossPostMeta else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I get this property may not be available in the Swift API, but is there a safer way (at least using a symbol, rather than a string) to do this?

Copy link
Contributor Author

@kean kean Feb 27, 2026

Choose a reason for hiding this comment

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

It's an interesting issue. The RCA is the following (generated):

Here's the problem: RemoteReaderCrossPostMeta is defined in Swift (WordPressKitModels), but the ObjC header in WordPressKitObjC only has @Class
RemoteReaderCrossPostMeta (a forward declaration). When Swift imports the WordPressKitObjC module, it can't resolve this forward-declared type to the
actual Swift class across module boundaries, so the crossPostMeta property is dropped from the Swift-visible interface.

I opted to fix it by converting RemoteReaderCrossPostMeta, a small class, back to ObjC so that the entire RemoteReaderPost object graph is in ObjC. I also added nullability annotations (everything nullable).

@kean kean force-pushed the task/convert-remote-post-mapper branch 2 times, most recently from da9c60c to 7f8a114 Compare February 27, 2026 13:46
@kean kean requested a review from crazytonyli February 27, 2026 13:47
@kean kean force-pushed the task/convert-remote-post-mapper branch from 7f8a114 to 985fa3a Compare February 27, 2026 13:54
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants