Update getMetrics types #10
Conversation
woubuc
left a comment
woubuc
left a comment
There was a problem hiding this comment.
Apologies for the wait, this have been a little hectic these days. Your changes look great! Just two small syntax details where I'm not sure why you changed them, see my comments.
| let url = endpoint | ||
| import { APIKey } from '../types'; | ||
|
|
||
| export const formatUrl = (apiKey : APIKey, endpoint : string[], query : Record<string, any> = {}) : string => { |
There was a problem hiding this comment.
Is there a particular reason you made this an arrow function? If not, I think it would be clearer if you change this back to a regular function notation.
There was a problem hiding this comment.
All other functions I've changed are arrow functions, so this would make it consistent
|
|
||
|
|
||
| export interface GetMetricsOptions { | ||
| export type GetMetricsOptions<Group extends MetricsGroup> = { |
There was a problem hiding this comment.
This is an object notation, so why should this be a type rather than an interface?
There was a problem hiding this comment.
I think using a consistent use of type (see other types) would be better
MichaelDeBoey
force-pushed
the
update-getMetrics-types
branch
from
4733667 to
5db6348
Compare
2 participants
As discussed in #9 (comment), I've updated the types of
getMetrics