[Part of #176153] Analytics client#179472
Conversation
|
/ci |
|
/ci |
|
/ci |
|
Pinging @elastic/kibana-core (Team:Core) |
jloleysens
left a comment
There was a problem hiding this comment.
Approving to unblock progress, overall looks good to me. I'll leave the decision about the test up to you!
| function isObject(value: unknown): value is Record<string, unknown> { | ||
| return typeof value === 'object' && value !== null; | ||
| } |
There was a problem hiding this comment.
Not sure it's worth the trouble, but what do you think about moving this to shared lib like @kbn/std?
There was a problem hiding this comment.
I thought about it. But we're thinking about porting this package to an independent NPM module. So I thought it might be better not to import other packages at this stage.
| private removeEmptyValues(context?: Partial<EventContext>) { | ||
| if (!context) { | ||
| private removeEmptyValues(context: unknown) { | ||
| if (!isObject(context)) { |
There was a problem hiding this comment.
Do we need a test for this new behaviour?
There was a problem hiding this comment.
It is tested (it tests if undefined is handled correctly). This was about fixing the types (and that forced me to add the type guard)
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: cc @afharo |
Summary
Part of #176153. Fixing the analytics client files.
For maintainers