Skip to content

Update access level for types that will go into WordPressData – Round 2 - #24348

Merged
mokagio merged 3 commits into
trunkfrom
mokagio/access-level-updates-continue
Mar 31, 2025
Merged

Update access level for types that will go into WordPressData – Round 2#24348
mokagio merged 3 commits into
trunkfrom
mokagio/access-level-updates-continue

Conversation

@mokagio

@mokagio mokagio commented Mar 28, 2025

Copy link
Copy Markdown
Contributor

Same rationale as #24326. Made possible by #24331. Part of #24165.

@mokagio
mokagio marked this pull request as draft March 28, 2025 00:06
@mokagio
mokagio force-pushed the mokagio/access-level-updates-continue branch from 46c698e to 3cd2d60 Compare March 28, 2025 00:41
@mokagio
mokagio changed the base branch from trunk to mokagio/decouple-round-2 March 28, 2025 00:41
Base automatically changed from mokagio/decouple-round-2 to trunk March 28, 2025 14:01
@mokagio
mokagio force-pushed the mokagio/access-level-updates-continue branch from 3cd2d60 to be865fc Compare March 30, 2025 20:55
@dangermattic

dangermattic commented Mar 30, 2025

Copy link
Copy Markdown
Collaborator
2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ This PR is assigned to the milestone 25.9. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@mokagio
mokagio force-pushed the mokagio/access-level-updates-continue branch from b0a0511 to 262b4b4 Compare March 31, 2025 02:18
mokagio added 3 commits March 31, 2025 13:21
All the changes are extracted from the "work bench" work from
#24242 with the
goal of reducing that PRs diff size to make it easier to review and
identify errors.
@mokagio
mokagio force-pushed the mokagio/access-level-updates-continue branch from 262b4b4 to f6df491 Compare March 31, 2025 02:22
@wpmobilebot

wpmobilebot commented Mar 31, 2025

Copy link
Copy Markdown
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 Number26914
VersionPR #24348
Bundle IDorg.wordpress.alpha
Commitf6df491
Installation URL3a5sdd3afn40o
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Mar 31, 2025

Copy link
Copy Markdown
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 Number26914
VersionPR #24348
Bundle IDcom.jetpack.alpha
Commitf6df491
Installation URL4stdfghm3e71g
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@mokagio
mokagio marked this pull request as ready for review March 31, 2025 03:54
@mokagio mokagio added this to the 25.9 milestone Mar 31, 2025
@mokagio mokagio added the Core Data Issues related to Core Data label Mar 31, 2025
@mokagio
mokagio requested review from crazytonyli and kean March 31, 2025 03:54
store.set(currentBuildVersion(), forKey: Self.lastSanitizationVersionNumber)

let entityNamesWithRequiredBlogProperties = [
// FIXME: Not sure why it's necessary to define [String] as the type. Without it, the NSFetchRequest<NSManagedObject>(entityName: entityName) below does not compile.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that's because there are multiple entityName definitions with mismatch APIs. For example, +[PostCategory entityName] returns an IUO in Swift, and the entityName on all NSManagedObject returns non-optional String. Changing the access level probably also changed how Swift compiler chooses which API to use.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the deep dive @crazytonyli !

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

Labels

Core Data Issues related to Core Data

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants