feat: Convert projectConfig module to TS #653
Conversation
yavorona
force-pushed
the
pnguen/convert-project-config-to-ts
branch
from
e10eda9 to
8acab1c
Compare
yavorona
force-pushed
the
pnguen/convert-project-config-to-ts
branch
from
67eb025 to
fa3a61d
Compare
optimizely-codereview-bot
added
🤖 12 hrs
🤖 16 hrs
🤖 20 hrs
🤖 24 hrs
and removed
🤖 8 hrs
🤖 12 hrs
🤖 16 hrs
🤖 20 hrs
labels
|
I reviewed #659 first and i see that you have already converted |
|
@zashraf1985 the #659 branch is based off of this branch. The plan was for this pr to be merged first and #659 to be rebased after that. Should have been more explicit about it, sorry! |
| * Determine for given experiment if event is running, which determines whether should be dispatched or not | ||
| * @param {ProjectConfig} configObj Object representing project configuration | ||
| * @param {string} experimentKey Experiment key for which the status is to be determined | ||
| * @return {boolean} True is the experiment is running |
| * @param {ProjectConfig} configObj Object representing project configuration | ||
| * @param {string} experimentKey Experiment key for which the status is to be determined | ||
| * @return {boolean} True is the experiment is running | ||
| * False is the experiment is not running |
| variableValue: string, | ||
| variableType: string, | ||
| logger: LogHandler | ||
| ): unknown { |
There was a problem hiding this comment.
Should return type be one of the variable types or'ed? like string | number | .... Just suggesting. this is also fine as is.
There was a problem hiding this comment.
We have been using unknown everywhere when dealing with variables of unknown type (example functions: getFeatureVariableForType, getFeatureVariableValueFromVariation , and getAllFeatureVariables, so I think it is best to stay consistent with what we have.
| * @returns {TryCreatingProjectConfigResult} | ||
| */ | ||
| export const tryCreatingProjectConfig = function( | ||
| config: TryCreatingProjectConfigConfig |
There was a problem hiding this comment.
Why defining this interface separately. if its not being reused anywhere or not being exposed outside, this can probably be done inline.? same for the return type. Just a suggestion, feel free to take it or leave it.
Summary
Convert
projectConfigmodule to TSTest plan
Existing unit tests