Selki/node sdk relay features/bandits handling #104
Conversation
| async resetSdk() { | ||
| const currentInstance = getInstance(); | ||
| currentInstance.stopPolling(); | ||
| await init({ |
There was a problem hiding this comment.
Interesting that forceReinitialize: true hasn't made it to node'sinit(), and it just always spins up a new instance.
There was a problem hiding this comment.
Oh sheeesh! I had no idea of this option's existence lmao. Will try using that instead.
| [AssignmentTypes.JSON_ASSIGNMENT_TYPE, 'getJSONAssignment'], | ||
| ]); | ||
|
|
||
| private castAssignmentResult(result, assignmentType: AssignmentTypes) { |
There was a problem hiding this comment.
Should we include type here?
result: AssignmentResult
Also, minor, but I'd consider having this method after where it called. That way the code reads from highest-level to lower level. Sort of like a newspaper.
| case AssignmentTypes.INTEGER_ASSIGNMENT_TYPE: | ||
| return parseInt(result, 10); | ||
| case AssignmentTypes.STRING_ASSIGNMENT_TYPE: | ||
| return result; |
There was a problem hiding this comment.
do we want String() or .toString() here?
There was a problem hiding this comment.
That's going to be more future proof for sure.
There was a problem hiding this comment.
Quick comment - this needs to be integrated into github actions:
https://github.com/Eppo-exp/sdk-test-data/blob/main/.github/workflows/test-sdk-packages.yml#L10
On its own it does not produce any meaningful testing. After integrating the github actions will compile the code from source, start the server and collect test results. Are you wanting to add that in this PR or a stacked one? I prefer this PR as it would make reviewing the code in it easier since we can see whether it passes.
@leoromanovsky I wanted to do a stacked one to avoid bigger PR but now that you mentioned that It does makes sense to keep it in one PR. I will tag you once more once I add everything regarding workflows. |
* test case for special characters * update obfuscated config * update names in description * include unevaluated allocation * update cyrillic language example * fix reason
No description provided.