Skip to content

Conversation

@mdholloway
Copy link
Contributor

@mdholloway mdholloway commented Jun 30, 2021

Splits the EventPlatformClient's Objective-C compatilibity layer into a bridging class which will remain in the app repository after the client library is consolidated with the other platform libraries as part of the work falling under T280182. This is needed because the standalone Swift client library will need to run on Linux, where the @objc annotation providing interoperability is not supported.

The bridging class will likely also be used to expose main app functionality to the client library once the library itself is extracted.

Note: Metrics Client is the new name for the Event Platform Client project.

Splits the EventPlatformClient's Objective-C compatilibity layer into
a bridging class which will remain in the app repository after the
client library is consolidated with the other platform libraries as
part of the work falling under T280182. This is needed because the
standalone Swift client library will need to run on Linux, where the
`@objc` annotation providing interoperability is not supported.

The bridging class will likely also be used to expose main app
functionality to the client library once the library itself is
extracted.

Note: Metrics Client is the new name for the Event Platform Client
project.
Copy link
Collaborator

@tonisevener tonisevener left a comment

Choose a reason for hiding this comment

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

@mdholloway This looks good! Just one small ask about MetricsClientObjectiveCCompat.


// MARK: Objective-C

public protocol MetricsClientObjectiveCCompat: BackgroundFetcher, PeriodicWorker {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't look like this protocol is used anywhere, can you remove it? Your extensions in MetricsClientBridge.swift should be enough to conform to BackgroundFetcher and PeriodicWorker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@tonisevener tonisevener merged commit 2650744 into wikimedia:main Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants