fix: Use .zip in archive path
#119
Conversation
|
Thanks @dnys1! Would you mind resolving conflicts? Then we'll take a look. |
Fixes dart-lang#118 Ensures that downloads of the Dart SDK zip file carry the `.zip` extension so that subsequent calls to `Expand-Archive` on Windows succeed. ## Notes - The diff of `index.mjs` is quite large. I believe this is due to the version of Node I'm using (`20.7.0`). Happy to use a different version to keep the diff small, but wasn't sure which one is currently used. This could be noted in `package.json` to ensure alignment with external contributions.
|
@mit-mit Done 😄 |
There was a problem hiding this comment.
Looks good, thanks!
Can you also add a new changelog entry to the changelog file? You can start a new v1.6.5-wip entry.
The diff of index.mjs is quite large. I believe this is due to the version of Node I'm using (20.7.0). Happy to use a different version to keep the diff small, but wasn't sure which one is currently used. This could be noted in package.json (under engines), if desired, to ensure alignment with external contributions.
fwiw I'm using v20.6.1. If you think that this will reduce diffs then PRs here would be welcome. We're mostly treating the dist/index.mjs file as opaque - we know it works if all the CI tests pass. Do you think specifying a specific version of node would help? I'd be worried about then remembering to rev that version periodically. Plus, many of the diffs in the file may be from the npm packages in use?
| await promiseToFuture<String>(toolCache.downloadTool(url)); | ||
| final archivePath = path.join( | ||
| // `RUNNER_TEMP` variable is guaranteed to be present. | ||
| // https://github.com/actions/toolkit/blob/5430c5d84832076372990c7c27f900878ff66dc9/packages/tool-cache/src/tool-cache.ts#L756 |
There was a problem hiding this comment.
| // https://github.com/actions/toolkit/blob/5430c5d84832076372990c7c27f900878ff66dc9/packages/tool-cache/src/tool-cache.ts#L756 | |
| // https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables |
| @@ -8,7 +8,7 @@ import 'dart:js_interop'; | |||
| external ToolCache get toolCache; | |||
|
|
|||
| @JS() | |||
| extension type ToolCache (JSObject obj) { | |||
| extension type ToolCache(JSObject obj) { | |||
There was a problem hiding this comment.
nit: can you confirm that this file is dart formatted?
Fixes #118
Ensures that downloads of the Dart SDK zip file carry the
.zipextension so that subsequent calls toExpand-Archiveon Windows succeed.Notes
index.mjsis quite large. I believe this is due to the version of Node I'm using (20.7.0). Happy to use a different version to keep the diff small, but wasn't sure which one is currently used. This could be noted inpackage.json(under engines), if desired, to ensure alignment with external contributions.Contribution guidelines:
dart format.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.