Add tool-cache #12
Add tool-cache #12
Conversation
| * @returns path to downloaded tool | ||
| */ | ||
| export async function downloadTool(url: string): Promise<string> { | ||
| return new Promise<string>(async (resolve, reject) => { |
There was a problem hiding this comment.
I don't think we need to construct a promise here manually, do we? If so, I think a comment is warranted. Keep in mind that async functions implicitly return a promise.
There was a problem hiding this comment.
I wrapped it so that I can resolve from within the stream callbacks. e.g.
file.on('open', async () => {
*do stuff*
resolve
})
If I just return normally there, it just returns from the callback. Is there an easier way to handle that? Added a comment for now
|
|
||
| dest = dest || (await _createExtractFolder(dest)) | ||
|
|
||
| if (IS_WINDOWS) { |
There was a problem hiding this comment.
I wonder if we should introduce a pattern for OS splitting—rather than this IS_WINDOWS branch, does it make sense to do something like:
IS_WINDOWS ? extractZipWin() : extractZipNix()Do you think this would improve readabilty of these branches?
There was a problem hiding this comment.
I like it here, not sure how useful I see it being in other places - I think most of these splits are pretty small amounts of code compared to this one.
I also ditched the ? : syntax in favor of traditional if else because it felt a little crowded with the awaits thrown in.
No description provided.