-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Ignore an Objective-C exception #24250
base: trunk
Are you sure you want to change the base?
Conversation
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr24250-9e6918d | |
Version | 25.8 | |
Bundle ID | org.wordpress.alpha | |
Commit | 9e6918d | |
App Center Build | WPiOS - One-Offs #11716 |
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr24250-9e6918d | |
Version | 25.8 | |
Bundle ID | com.jetpack.alpha | |
Commit | 9e6918d | |
App Center Build | jetpack-installable-builds #10745 |
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.
Alright!
Sorry it took me a while to review this, but I wanted to verify it locally (just in case!) using the steps from #24247
All good!
Approving to unblock, my question regarding the scope to which we should wrap possible exceptions is not a blocker at all.
try? WPException.objcTry { | ||
self.mySiteViewController = self.makeMySiteViewController() | ||
self.navigationController.viewControllers = [self.rootContentViewController] | ||
} |
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.
I suppose we consider safe to blanket ignore any exception here because we are confident they would arise only when running unit tests due to the many stateful assumption in the app at the moment.
Is that safe?
How would you feel about running this only if NSClassFromString("XCTestCase") != nil
?
Fixes #24247.
See the inline code comment for details.
Regression Notes
Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: