-
Notifications
You must be signed in to change notification settings - Fork 222
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
Change to use Mach exception handler by default #637
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #637 +/- ##
===========================================
- Coverage 92.9% 92.85% -0.05%
===========================================
Files 207 207
Lines 14508 14510 +2
Branches 841 841
===========================================
- Hits 13478 13473 -5
- Misses 1017 1024 +7
Partials 13 13
Continue to review full report at Codecov.
|
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.
Very minor comments.
* The SDK will not check if the app is running in an AppStore environment or if a debugger is attached because some | ||
* developers chose to do both at their own risk. | ||
* `[MSCrashes disableMachExceptionHandler]`; | ||
* `[MSMobileCenter start:@"YOUR_APP+_ID" withServices:@[[MSCrashes class]]];` |
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.
"YOUR_APP_ID", a +
sign sneaked into it ^^.
* `[MSCrashes enableMachExceptionHandler]`; | ||
* `[MSMobileCenter start:@"YOUR_APP+_ID" withServices:@[[MSCrashes class]]];` | ||
* `MSCrashes.disableMachExceptionHandler()` | ||
* `MSMobileCenter.start("YOUR_APP+_ID", withServices: [MSAnalytics.self, MSCrashes.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.
Same thing here: "YOUR_APP_ID".
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 is a breaking change and we need to bump the version because of that.
Good point @TroubleMakerBen. I also updated the version. |
There will be a separate doc PR in doc repo.