Remove request and request-promise-native in favour of got #3974
Conversation
|
Because I think that got's API is better. |
|
|
||
| for (const detectorClass of this.registry) { | ||
| const detector = new detectorClass(cluster); | ||
| return [detector.key, await detector.detect()] as const; |
There was a problem hiding this comment.
What means as const and why do we need it here?
There was a problem hiding this comment.
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#const-assertions
It is the way to say that you want a tuple of distinct types instead of an array of the union of all possible types.
| return Object.fromEntries( | ||
| iter.map( | ||
| Object.entries(results), | ||
| ([key, { value }]) => [key, value] | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Why do we need iter.map here? Is it different somehow from:
return Object.fromEntries(
Object.entries(results).map(([key, { value }]) => [key, value])
);There was a problem hiding this comment.
Doesn't allocate a new array. That is all (since fromEntries wants an iterable). I can change it if you would like.
Good point btw. Or just use |
@Nokel81 I meant that we are using https://github.com/lensapp/lens/blob/master/src/common/k8s-api/json-api.ts#L25 |
We are already using |
|
@jakolehm Here is a comparison between the two (yes I admit it is from the GOT side) https://github.com/sindresorhus/got#comparison |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
I have been using got in other projects and it's fine choice but does not really warrant yet-another library included if we already have node-fetch. I'm fine with this PR because it removes deprecated |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Nokel81
force-pushed
the
issue-459
branch
2 times, most recently
from
16143f7 to
e0f66e6
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Sebastian Malton <[email protected]>
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
|
I am going to close this PR, instead move to |
Signed-off-by: Sebastian Malton [email protected]
fixes #459