Repository navigation
Add tool-cache - #12
Conversation
jclem
left a comment
jclem
left a comment
There was a problem hiding this comment.
Added some little notes here and there. Great work, thank you!
I can't get these tests to pass—do I have to run these in a container for them to work?
| * @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.
* Add tool-cache * Format * Fix linux bug * Update readme * Package zip/unzip * Reorganize * Format * unzip somehow got corrupted, fixing * Resolve remaining todos * Don't log everything * Pass error codes * linting * Branding and nits * Fix for mac * clean up windows/nix logic * Clean up tests * Export HTTPError * Add option for custom 7z tool * Lint/format * Dont wipe temp/tool directories
2 participants
No description provided.