Skip to content
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

Create structured BugsnagThread class #532

Merged
merged 3 commits into from
Apr 9, 2020
Merged

Conversation

fractalwrench
Copy link
Contributor

Goal

Creates a publicly accessible BugsnagThread class which holds fields in properties, rather than an NSDictionary.

This is used as a readonly array property on BugsnagEvent, where the user can alter thread information in callbacks. The changeset relies on #528 which has not yet been merged.

Changeset

  • Add properties to BugsnagThread which can be edited by the user. Note that these deviate from the spec, specifically thread.name whose value will always be nil by default

  • Updated BugsnagEvent thread generation to occur in constructor rather than in the toJson method

  • Updated BugsnagEvent JSON serialization to use BugsnagThread internally

Tests

Added unit tests to verify that threads can be parsed from a dictionary/serialized to JSON. Added additional coverage to ensure that thread information is enhanced with isPC and isLR. Relied on existing unit test coverage to verify the event JSON remains unchanged.

@fractalwrench fractalwrench force-pushed the v6-structured-thread branch from 8bdc07b to 062f198 Compare April 8, 2020 13:52
@fractalwrench fractalwrench force-pushed the v6-structured-stacktrace branch from 413e92b to c50dbaa Compare April 8, 2020 15:12
@fractalwrench fractalwrench force-pushed the v6-structured-thread branch from 062f198 to c1cce43 Compare April 8, 2020 15:32
@fractalwrench fractalwrench marked this pull request as ready for review April 8, 2020 15:32
Source/BugsnagEvent.m Outdated Show resolved Hide resolved
Source/BugsnagThread.m Outdated Show resolved Hide resolved
Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from a couple of nits and querying the necessity of a parameter, a few comments would help my understanding. Other than that, Looks good!

@fractalwrench fractalwrench force-pushed the v6-structured-stacktrace branch from c50dbaa to 8402c86 Compare April 9, 2020 10:32
@fractalwrench fractalwrench force-pushed the v6-structured-thread branch from c1cce43 to cbed144 Compare April 9, 2020 10:48
Copy link
Contributor

@robinmacharg robinmacharg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fractalwrench fractalwrench changed the base branch from v6-structured-stacktrace to v6 April 9, 2020 12:33
@fractalwrench fractalwrench merged commit 35ffa96 into v6 Apr 9, 2020
@fractalwrench fractalwrench deleted the v6-structured-thread branch April 9, 2020 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants