-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Crashlytics GDT fill in Exception part of Proto #4894
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
Crashlytics GDT fill in Exception part of Proto #4894
Conversation
@@ -26,6 +26,8 @@ | |||
#import <nanopb/pb_decode.h> | |||
#import <nanopb/pb_encode.h> | |||
|
|||
const NSUInteger IMPORTANCE_FOR_EXCEPTION = 0; |
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.
The naming for this may be confusing since the importance for exceptions be non-zero. Take a look at Protobuf.scala:384.
Maybe this should be DEFAULT_IMPORTANCE_FOR_EXCEPTION?
@@ -26,6 +26,8 @@ | |||
#import <nanopb/pb_decode.h> | |||
#import <nanopb/pb_encode.h> | |||
|
|||
const NSUInteger IMPORTANCE_FOR_EXCEPTION = 0; |
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.
In FIRCLSRecordThread.m:21, we define IMPORTANCE_IN_CRASHED_THREAD. Does it make more sense to have this const be defined FIRCLSRecordFrame? FIRCLSRecordFrame could also expose an importance property to abstract the logic from this adapter 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.
Yeah that probably makes more sense to organize it all there
Exception
part of the app execution.signalProto
Does not fill in symbolicated files or importance for symbolicated.