feat: add OptimizelyConfig API #274
Conversation
jaeopt
requested review from
thomaszurkan-optimizely
and removed request for
a team
There was a problem hiding this comment.
I think there is an opportunity to do some really cool things here. why give access to maps that can go stale? why not instead support subscripting and make it as dynamic and realtime as possible? so instead of optimizely config as an object it is just a set of methods that access the project config (i.e. client.config.features["featureName"] as well as client.config.featureKeys[i]).
I think the main reason you would want it like this is to discourage holding onto potentially stale data.
| @@ -16,12 +16,47 @@ | |||
|
|
|||
| import Foundation | |||
|
|
|||
| struct FeatureFlag: Codable, Equatable { | |||
| struct FeatureFlag: Codable, Equatable, OptimizelyFeature { | |||
| static func == (lhs: FeatureFlag, rhs: FeatureFlag) -> Bool { | |||
There was a problem hiding this comment.
why not have these as extensions?
Sources/Data Model/Variable.swift
Outdated
| @@ -16,7 +16,35 @@ | |||
|
|
|||
| import Foundation | |||
|
|
|||
| struct Variable: Codable, Equatable { | |||
| struct Variable: Codable, Equatable, OptimizelyVariable { | |||
There was a problem hiding this comment.
i think this would be cleaner as an extension.
| @@ -128,7 +128,7 @@ class DefaultDatafileHandler: OPTDatafileHandler { | |||
| completionHandler(result) | |||
|
|
|||
| // avoid event-log-message preparation overheads with closure-logging | |||
| self.logger.d({ response.debugDescription }) | |||
| //self.logger.d({ response.debugDescription }) | |||
There was a problem hiding this comment.
should we remove this?
| /// - Returns: a snapshot of public project configuration data model | ||
| /// - Throws: `OptimizelyError` if SDK is not ready | ||
| @available(swift, obsoleted: 1.0) | ||
| @objc(getOptimizelyConfigWithError:) |
There was a problem hiding this comment.
why is this not a property?
There was a problem hiding this comment.
I guess you're asking if we can use property for "optimizelyConfig" instead of get function. It's a possible option, but we can integrate error throw/return and also in this way, more consistent with other APIs.
|
Tom, it's an interesting idea to let dynamically access updated data with subscripts. I thought about it but will put aside for now. It'll make sync issue complicated. We need to take a snapshot to provide data consistency. A consistency of API with other SDKs is also considered. |
| let task = session.uploadTask(with: request, from: event.body) { (_, response, error) in | ||
| self.logger.d(response.debugDescription) | ||
| let task = session.uploadTask(with: request, from: event.body) { (_, _, error) in | ||
| //self.logger.d(response.debugDescription) |
No description provided.