Skip to content

Convert BlogFeature to Swift#25296

Merged
kean merged 1 commit intotrunkfrom
task/convert-blog-features
Feb 26, 2026
Merged

Convert BlogFeature to Swift#25296
kean merged 1 commit intotrunkfrom
task/convert-blog-features

Conversation

@kean
Copy link
Contributor

@kean kean commented Feb 24, 2026

It's impractical to test it, so I carefully reviewed every line that changed. There are few existing unit tests for the more advanced features as well.

There are still a couple of simplifications that could be made. I plan to address them separately.

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

1 Warning
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@sonarqubecloud
Copy link

}

if ([self.blog supports:BlogFeatureWPComRESTAPI] && self.blog.isAdmin) {
if ([self.blog supports:BlogFeatureWpComRESTAPI] && self.blog.isAdmin) {
Copy link
Contributor Author

@kean kean Feb 24, 2026

Choose a reason for hiding this comment

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

It's automatic conversion from .wpComRESTAPI – should be fine. The ObjC code is legacy code, and it would be ideal to convert it later as well.

@kean kean requested a review from crazytonyli February 24, 2026 23:46
@kean kean enabled auto-merge February 24, 2026 23:46
@wpmobilebot
Copy link
Contributor

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 Number31114
VersionPR #25296
Bundle IDorg.wordpress.alpha
Commit271a063
Installation URL106rms0n1j2c0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

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

@wpmobilebot
Copy link
Contributor

🤖 Build Failure Analysis

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

@kean kean requested a review from jkmassel February 25, 2026 13:40
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

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

Comprehensive Code Review

I did a line-by-line comparison of every BlogFeature case (old ObjC vs new Swift), verified all call sites, and built/tested locally. This is a clean, well-executed conversion. All 45 enum cases are faithfully translated, all removed methods have no external callers, and the @objc bridging names are correct (with the WPComRESTAPIWpComRESTAPI rename properly propagated to all 6 ObjC call sites).

One behavioral change worth noting

hasRequiredJetpackVersion has a subtle behavioral change when jetpack?.version is nil:

Old ObjC:

[self.jetpack.version compare:requiredJetpackVersion options:NSNumericSearch] != NSOrderedAscending

When jetpack or version is nil, ObjC nil-messaging returns NSOrderedSame (0), so NSOrderedSame != NSOrderedAscendingYES. The old code incorrectly returned true for sites with no Jetpack version info.

New Swift:

guard supportsRestAPI, !isHostedAtWPcom,
      let version = jetpack?.version else {
    return false
}

The guard let correctly returns false when version is nil.

This is almost certainly a latent bug fix (a nil version should never satisfy a version requirement), and the real-world impact is limited since most affected features (.contactInfo, .videoPressV5, .facebookEmbed, etc.) have || isHostedAtWPcom fallbacks. Just flagging since the PR description says the conversion is faithful — this is actually more correct than the original.

Verified locally

  • WordPress scheme: BUILD SUCCEEDED
  • WordPressTest suite: 69 tests, all passed
  • WordPressShareExtension: BUILD SUCCEEDED

Nice cleanup! 👍

@kean kean added this pull request to the merge queue Feb 26, 2026
Merged via the queue into trunk with commit 255cf6a Feb 26, 2026
29 of 34 checks passed
@kean kean deleted the task/convert-blog-features branch February 26, 2026 16:09
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