-
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
Make metadata interface consistent across config client and event #513
Make metadata interface consistent across config client and event #513
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.
Happy with the overall plan, and the public signature changes made in this changeset.
Making BugsnagMetadata
conform to the protocol would be good, as the implementation for adding/getting/clearing metadata could be contained within there. BugsnagClient
and BugsnagEvent
could then hold a property of type BugsnagMetadata
and forward on any method calls to it, rather than implementing each call in multiple classes. We've done something similar on Android here.
- Added NS_SWIFT_NAMES to protocol. - Added class-level MetadataStore protocol for Bugsnag. - Removed header declarations. - Added missing method to Bugsnag - (temporary)Corrected source of metadata in a couple of places. - Corrected interface mutability of dictionaries. - Fixed up tests
Added deep dict and array copy to enable metadata copying
- Removed erroneous parse code - Updated tests - Made Event initialisation allow nullable Metadata (new object is subbed) - Fixed Bugsnag top-level addMetadata methods - Removed <NSMutableCopying> from BugsnagMetadata
2cdc05f
to
7eb62ba
Compare
…erface-consistent-across-Config-Client-and-Event # Conflicts: # Bugsnag.podspec.json # Source/Bugsnag.m # Source/BugsnagClient.h # Source/BugsnagClient.m
…erface-consistent-across-Config-Client-and-Event # Conflicts: # Source/Bugsnag.h
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 looks good - and is a hard thing to change. I've left a few inline comments but think this is basically there and will be ready for merging once they've been resolved.
- Added docs - Renamed <BugsnagMetadataStore> parameters - Removed unnecessary deepCopies and mutating modifiers
0707d65
to
79cb452
Compare
…erface-consistent-across-Config-Client-and-Event # Conflicts: # Bugsnag.podspec.json
Goal
Implement a consistent metadata interface across Config, Client and Event classes.
Design
A
BugsnagMetadataStore
instance-level protocol is defined, as well as a class-level version of the same, containing the following methods:addMetadata:toSection:
addMetadata:withKey:toSection:
getMetadataFromSection:
getMetadataFromSection:withKey:
clearMetadataFromSection:
clearMetadataFromSection:withKey:
Bugsnag
,BugsnagClient
,BugsnagEvent
,BugsnagConfig
andBugsnagMetadata
classes are all changed to conform to this.Internal dictionary metadata implementations are replaced with
BugsnagMetadata
objects.Changeset
A wide range of files, not limited to those mentioned above, since many references to object metadata also needed updated. Test apps have also been updated.
Tests
No significant new tests added, but many changes to existing ones to adopt the new non-dictionary access syntax.
Review
Outstanding Questions
master
for fixes,next
forfeatures)