Replace got with fetch #38
Conversation
| "semver": "^7.1.1", | ||
| "yargs": "^16.2.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/node": "^20.14.11", |
There was a problem hiding this comment.
There was in implicit dependency on these types which was satisfied by bringing in got. Why it had it as a prod dep I don't think I understand, but removing got broke the type generation.
There was a problem hiding this comment.
it should be node 18, given engines.node, no?
|
@wesleytodd can we move on with this? Also, I think this is not a breaking change, is it? |
|
I am fine doing it as minor, and yeah we can move on it, I just have not had time to test and make sure everything was good, nor do the release. Do you have perms do to all that? If not, and I am this delayed again circling back feel free to ping me in slack. |
| }).json() | ||
| const cached = cache.get('schedule') | ||
| if (cached) { | ||
| return Promise.resolve(cached) |
There was a problem hiding this comment.
if this was made an async function then you wouldn't need the promise wrapper
| }).json() | ||
| const cached = cache.get('versions') | ||
| if (cached) { | ||
| return Promise.resolve(cached) |
| "semver": "^7.1.1", | ||
| "yargs": "^16.2.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/node": "^20.14.11", |
There was a problem hiding this comment.
it should be node 18, given engines.node, no?
|
I am probably unable to make these updates, but I agree with them. If someone wants to take this over feel free. |
While the tests pass and it all seems fine, the
cachekey used to be aMapforgotto use. Now it is just a simple in memory lookup. Same api to pass, but technically someone could have been looking at the contents of the map they passed in, so this could break them so marking it as major.