-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add dedicated secrets file for Reader #24463
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
Conversation
As a result, the secrets files for the other apps updated too. That's because since the last update we removed the App Center API key from them. See also 9348997
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 27363 | |
Version | PR #24463 | |
Bundle ID | org.wordpress.alpha | |
Commit | 910775d | |
Installation URL | 6fo7o14qe0390 |
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 27363 | |
Version | PR #24463 | |
Bundle ID | com.jetpack.alpha | |
Commit | 910775d | |
Installation URL | 7b22ifk2bj7kg |
@@ -3,4 +3,4 @@ | |||
|
|||
WP_PUSH_NOTIFICATION_APP_ID = TBD | |||
WP_BUILD_CONFIGURATION = alpha | |||
WP_APP_URL_SCHEME = TBD | |||
WP_APP_URL_SCHEME = wpreader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value could also be specialized by build configuration, e.g.:
WP_APP_URL_SCHEME = wpdebug |
WP_APP_URL_SCHEME = wordpress |
but I'm not sure if we need to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WP_APP_URL_SCHEME
should be one for each app identifier. If I'm not mistaken, debug and release builds share the same app identifier, but alpha builds use a different one.
We need to use the bundle to the class, otherwise UIKit will look for the storyboard in the app (Reader) when the file is in Keystone.
@@ -32,9 +32,12 @@ | |||
"file": "iOS/JPiOS/Jetpack-Secrets.swift", | |||
"destination": "~/.configure/wordpress-ios/secrets/Jetpack-Secrets.swift", | |||
"encrypt": true | |||
}, | |||
{ | |||
"file": "iOS/Reader/Reader-Secrets.swift", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something not obvious to me was that the source file cannot be called Secrets.swift
, even though the Reader prefix seems redundant in a folder called Reader.
The reason for that is that when configure
encrypts the files locally, it uses the file name. And given we already have a source file called Secrets.swift
we cannot name this new one that.
@@ -108,7 +108,7 @@ final class ReaderTabViewController: UITabBarController, UITabBarControllerDeleg | |||
} | |||
|
|||
private func makeNotificationsViewController() -> UIViewController { | |||
let notificationsVC = UIStoryboard(name: "Notifications", bundle: nil) | |||
let notificationsVC = UIStoryboard(name: "Notifications", bundle: Bundle(for: Self.self)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#24464 follows up to this and addresses all such accesses for the notifications storyboard.
This is so we can make it use the bundle of the class conforming to it instead of using `.main`, which fails if the class and its nib are in a framework (Keystone)
|
As a result, the secrets files for the other apps updated too. That's because since the last update we removed the App Center API key from them (see also
9348997)
Part of https://linear.app/a8c/issue/CMM-260/configure-reader-target
Note
The only Reader-specific values currently tracked are the WordPress.com app id and the Sentry DSN