-
Notifications
You must be signed in to change notification settings - Fork 130
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
fix(oom): Report short version as app.version #349
Conversation
@@ -214,7 +214,7 @@ - (NSMutableDictionary *)generateCacheInfoWithConfig:(BugsnagConfiguration *)con | |||
app[@"id"] = systemInfo[@BSG_KSSystemField_BundleID] ?: @""; | |||
app[@"name"] = systemInfo[@BSG_KSSystemField_BundleName] ?: @""; | |||
app[@"releaseStage"] = config.releaseStage; | |||
app[@"version"] = systemInfo[@BSG_KSSystemField_BundleVersion] ?: @""; | |||
app[@"version"] = systemInfo[@BSG_KSSystemField_BundleShortVersion] ?: @""; |
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.
Do we want to add the bundle version in as well? Think it would something like
app[@"bundleVersion"] = systemInfo[@BSG_KSSystemField_BundleVersion] ?: @"";
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.
Yeah - having both makes sense 👍
@@ -155,7 +155,7 @@ - (BOOL)computeDidOOMLastLaunchWithConfig:(BugsnagConfiguration *)config { | |||
NSString *osVersion = [BSG_KSSystemInfo osBuildVersion]; | |||
NSDictionary *appInfo = [[NSBundle mainBundle] infoDictionary]; | |||
NSString *appVersion = | |||
[appInfo valueForKey:(__bridge NSString *)kCFBundleVersionKey]; | |||
[appInfo valueForKey:@BSG_KSSystemField_BundleShortVersion]; |
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.
If this is being used to detect if the app has the same version and hence report the OOM, should this be on app version and bundle version rather than just app version?
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.
That's a good question. The safest thing is to check both, so I'll make it do that.
Add bundleVersion as separate field
3ad37fc
to
cfeb982
Compare
Out-of-memory reports are incorrectly reporting the build number as the version number. This change fixes that error and adds a few assertions for it in OOM report tests.