-
Notifications
You must be signed in to change notification settings - Fork 718
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
Move demangle filter to an optional module #540
Conversation
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.
Great! Now we will be more in line with the principle of single responsibility!
I'm also curious about how users will install KSCrash by default, as they might have expected that demangling worked by default. It seems like most of the users would still need it, so I'm wondering how we can make that as easy as possible for them. |
It's a valid concern. I've added demangling to Installations API and made it enabled by-default. So those who were using installations can keep this functionality without extra efforts. And I've updated the README to highlight that this is now not a default behavior. Also, we'll need a "migration guide" for 2.0 anyway as there are other breaking changes and we need to highlight the expected app code updates. |
* Move demangle to an optional filter * Fix code * Fix package * Add to sample * Fix format * Fix test * Add privacy info * Add subspec * Add demangling to installations * Add tests * Fix Swift demangle returning non-nil on failure * Add scheme * Add protocol pragma mark * Update README * Fix format * Fix missing import * Fix README
Demangling is not a requirement for crash reports that are later passed through the symbolicator (with dSYMs).
To reduce minimal library size and exclude parts of swift/llvm code base, this module becomes optional.