Remove AppEnvironment from WordPressOrgRestApi#24351
Merged
Conversation
Collaborator
mokagio
added a commit
that referenced
this pull request
Mar 28, 2025
I noticed this unused property while working on #24351. I'll acknowledge that we should ditch `AppEnvironment` altogether, but short of time to address that, this at least reduces its surface area.
Contributor
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 26829 | |
| Version | PR #24351 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 49a26df | |
| Installation URL | 1u92aeunribk8 |
Contributor
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 26829 | |
| Version | PR #24351 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 49a26df | |
| Installation URL | 0p4rci2uo28ao |
kean
approved these changes
Mar 28, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 28, 2025
I noticed this unused property while working on #24351. I'll acknowledge that we should ditch `AppEnvironment` altogether, but short of time to address that, this at least reduces its surface area.
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 1, 2025
* Configure `Package.swift` with WordPressData framework deps * Add `WordPress-Swift.h` to WordPressData to work around build fail * Add prefix header * Add all required sources to WordPressData * Add required imports to Swift layer * Extract `defaultWordPressComAccountRestAPI` method to WordPressData Only the method, not the rest of the extension to avoid dependencies galore. * Add `Blog+SelfHosted` to WordPressData, including dependencies * Hack `ContextManager.swift` to compile * Replicate Keystone preprocessor macro hack for WordPressData-Swift.h * Add `Media+Blog.swift` to WordPressData to compile `PostHelper.m` * Add `RemotePost+Metadata` source to WordPressData for `PostHelper.m` * Add `PostServiceRemoteFactory` to WordPressData to fix `PostService.m` * Add `PostHelper+JetpackSocial` to WordPressData to fix `PostHelper.m` * Add `WordPressOrgRestApi+WordPress` to WordPressData Made possible by #24351 * Add `.m` file in WordPressData only to config CocoaLumberJack * Add some `import CocoaLumberJackSwift` required by WordPressData * Use `_private_wordPressComRestApi` in `Blog+WordPressComRestAPI.swift` to build WordPressData * Add CocoaLumberjack as a dependency to the extensions to fix compilation Otherwise, we'd get: ``` Undefined symbols for architecture arm64: "_OBJC_CLASS_$_DDLog", referenced from: in AppExtensionsService.o ld: symbol(s) not found for architecture arm64 clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` * Remove `WordPress-Swift.h` hack from WordPressData * Add `Blog+Lookup` to WordPressData * Add `WordPress.xcdatamodeld` to WordPressData * Duplicate test infra in WordPressData in order to run `BlogTests` * Remove unnecessary `CocoaLumberJackConfiguration.h` header We don't need the header, the implementation file is enough for the CocoaLumberJack shenanigans to take place. * Remove WordPressData types from WordPress and Jetpack Not Keystone yet because I don't know if that compiles okay to verify the change. * Update imports after removing WordPressData files from WP and Jetpack * Disambiguate `Notification` usages — `/: Notification\)/` * Disambiguate `Notification` usages — `/: Notification$/` * Disambiguate `Notification` usages — `/, Notification(?!s)/` * Disambiguate `Notification` usages — `/as\? Notification/` * Disambiguate `Notification` usages — `/ Notification \{/` * Disambiguate `Notification` usages — `/ Notification\?/` * Disambiguate `Notification` usages — `/ Notification,$/` * Disambiguate `Notification` usages — `/\[Notification\]/` * Disambiguate `Notification` usages — `/ Notification\:/` * Disambiguate `Notification` usages — `/: Notification,/` * Disambiguate `Notification` usages — `/\(Notification\)/` * Disambiguate `Notification` usages — `/Notification else/` * Disambiguate `Notification` usages — `/Notification\!/` * Disambiguate `Notification` usages — `/as\? Notification/` * Add CocoaLumberjack to notification service extensions * Update imports after `Notification` disambiguation * Replace all `#import` of headers now in WordPressData with `@import WordPressData` * Work around (temporarily?!) a build failure after the WordPressData move * Remove unused `AccountService` import from `Blog.m` It matters because once `Blog` is in WordPressData, `AccountService` will no longer be available. * Move some `MenuItem` imports from `.h` to `.m` `MenuItem` was not used in the interface. Moving the import in the implementation will make it cleaner to update once WordPressData is integrated. * Remove WordPressData types from `WordPress-Bridging-Header.h` * Address module verifier warnings in WordPressData * Extract Swift-dependant logic from `Theme` * Make `Media` extension `public` * Remove unused WordPressData import from NotificationServiceExtension * Add CocoaLumberjack to extensions Jetpack uses to avoid compile failure Undefined symbol: _OBJC_CLASS_$_DDLog * Make WordPressData Swift API available to ObjC framework consumers * Add `testable import WordPressData` to unit tests * Update `WordPress.Notification` to `WordPressData.Notification` in tests * Add more required `@testable import WordPressData` Via ``` find WordPress/WordPressTest \ -name "*.swift" \ -type f \ -exec perl -i -0pe 's/(@testable import WordPress\n)(\nclass (\w+): CoreDataTestCase \{)/\1\@testable import WordPressData\n\nclass \3: CoreDataTestCase {/g' \ {} \; ``` and ``` find WordPress/WordPressTest \ -name "*.swift" \ -type f \ -exec perl -i -0pe 's/(@testable import WordPress\n)(\nfinal class (\w+): CoreDataTestCase \{)/\1\@testable import WordPressData\n\nfinal class \3: CoreDataTestCase {/g' \ {} \; ``` * Add WordPressData as a WordPressTests dependency * Add missing dependencies to WordPressTesting post WordPressData addition * Load model from the `ContextManager` bundle in the unit tests * Replace `AppEnvironment` with `WordPressComRestApi.apiBaseURL` in RestAPI Same rationale as 09990b8 * No longer access `_private_wordPressComRestApi` in `Blog` * Fix WordPressData linkage in Keystone and move sources * Update access of types required by Keystone for WordPressData link * Explicitly add WebKit as a Keystone dependency Triggered by a few errors like ``` In file included from /Users/gio/Developer/a8c/wpios/WordPress/Classes/ViewRelated/Suggestions/SuggestionsTableViewCell.m:3: /Users/gio/Developer/a8c/wpios/DerivedData/WordPress/Build/Intermediates.noindex/WordPress.build/Debug-iphonesimulator/Keystone.build/DerivedSources/Keystone-Swift.h:295:9: fatal error: module 'WebKit' not found 295 | @import WebKit; | ~~~~~~~^~~~~~ ``` which I got after linking WordPressData against Keystone and moving all the sources there. * Revert `CookieJar` `public` access level Apparently unnecessary, see #24420 (comment) * REVERT ME - Only run WordPress tests * Switch all models to use "Global Namespace" Via ``` sed -i '' -E 's/representedClassName="(WordPress)?(\.)?([^"]+)"/representedClassName=".\3"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ 155.xcdatamodel/contents ``` which switched to "Current Product Module" but resulted in unit tests failure in WordPressData, followed by ``` ➜ sed -i '' -E 's/representedClassName="\.([^"]+)"/representedClassName="\1"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ 155.xcdatamodel/contents ``` * Replace all string values passed to `forEntityName` with `entityName()` Done via ``` find . \ \( -path './DerivedData' -o -path './.git' -o -path './vendor' \) \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: "([A-Za-z_][A-Za-z0-9_]*)"/forEntityName: \1.entityName()/g' {} + ``` * Replace all `NSStringFromClass` values passed to `forEntityName` Done via ``` find . \ \( -path './DerivedData' -o -path './.git' -o -path './vendor' \) \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: NSStringFromClass\(([A-Za-z_][A-Za-z0-9_]*)\.self\)/forEntityName: \1.entityName()/g' {} + ``` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Replace all `Self.classNameWithoutNamespaces()` with `entityName()` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Replace all `classNameWithoutNamespace()` calls with `entityName()` Done via ``` find . \( -path './DerivedData' -o -path './.git' -o -path './vendor' \) \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: ([A-Za-z_][A-Za-z0-9_]*)\.classNameWithoutNamespaces\(\)/forEntityName: \1.entityName()/g' {} + ``` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Fix WordPressData runtime issues (#24494) * Move remaining types to WordPressData * Use Current Module for Swift-only types * Fix ReaderTeamTopic compilation * Add missing class names * Remove unused SiteInfo * Add Bundle.wordPressData and update DataMigratorTests * Update CoreDataMigrationTests * Fix unit tests (AppEnvironment required) * Add `BlobEntity` to WordPressData * Address straightforward `FIXME` notes --------- Co-authored-by: Alex Grebenyuk <alex.grebenyuk@automattic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows up on the RFC from #24333, part of #24165.