-
Notifications
You must be signed in to change notification settings - Fork 19
[FME-12340] - Metadata - Managers #812
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
base: Metadata_baseline
Are you sure you want to change the base?
Conversation
SDK Readiness Test Results ✅
Legend: 🟢 significant improvement | 🔴 significant regression | ⚪ inconclusive (p≥0.05) ios-test-mock
|
SDK Readiness Test Results ✅
Legend: 🟢 significant improvement | 🔴 significant regression | ⚪ inconclusive (p≥0.05) ios-test-mock
|
SDK Readiness Test Results ✅
Legend: 🟢 significant improvement | 🔴 significant regression | ⚪ inconclusive (p≥0.05) ios-test-mock
|
| import Foundation | ||
|
|
||
| public typealias SplitAction = () -> Void | ||
| public typealias SplitActionWithMetadata = (EventMetadata) -> Void |
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.
Let's make sure to remove this one.
| public typealias SplitActionWithMetadata = (EventMetadata) -> Void |
|
|
||
| @objcMembers public final class SplitEventWithMetadata: NSObject, Sendable { | ||
| let type: SplitEvent | ||
| let metadata: EventMetadata? | ||
|
|
||
| @available(*, unavailable) | ||
| override init() { | ||
| fatalError("Use SDK-provided instances only") | ||
| } | ||
|
|
||
| internal init(type: SplitEvent, metadata: EventMetadata? = nil) { | ||
| self.type = type | ||
| self.metadata = metadata | ||
| } | ||
| } |
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 one was replaced by the event specific metadata classes, I think.
| @objcMembers public final class SplitEventWithMetadata: NSObject, Sendable { | |
| let type: SplitEvent | |
| let metadata: EventMetadata? | |
| @available(*, unavailable) | |
| override init() { | |
| fatalError("Use SDK-provided instances only") | |
| } | |
| internal init(type: SplitEvent, metadata: EventMetadata? = nil) { | |
| self.type = type | |
| self.metadata = metadata | |
| } | |
| } |
|
|
||
| protocol SplitEventsManager: AnyObject, Sendable { | ||
| func register(event: SplitEvent, task: SplitEventTask) | ||
| func notifyInternalEvent(_ event: SplitInternalEvent) |
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.
Seems this one could be marked as deprecated so we remove it later on. From what I see, we're only using SplitInternalEventWithMetadata now.
| func notifyInternalEvent(_ event: SplitInternalEvent) | |
| @available(*, deprecated, message: "Notify with metadata") | |
| func notifyInternalEvent(_ event: SplitInternalEvent) |
| /// - The specific flags affected | ||
| /// | ||
| @objc public class SdkUpdateMetadata: NSObject, EventMetadata { | ||
| @objcMembers public final class SdkUpdateMetadata: NSObject, EventMetadata { |
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.
Is extending EventMetadata needed? It would be great to not have EventMetadata as public (or better yet, not have it).
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.
We may have to add some explicit addEventsListener tests.
SDK Readiness Test Results ✅
Legend: 🟢 significant improvement | 🔴 significant regression | ⚪ inconclusive (p≥0.05) ios-test-mock
|
iOS SDK
What did you accomplish?
EventsManager updated.
How do we test the changes introduced in this PR?
Extra Notes
Tests on Next PR