Feature: Allow for cli version specification #253
Conversation
|
@microsoft-github-policy-service agree company="M Science LLC" |
common/src/dev-container-cli.ts
Outdated
| }); | ||
| return exitCode === 0; | ||
| const {exitCode, stdout} = await exec(getSpecCliInfo().command, ['--version'], {}); | ||
| return exitCode === 0 && stdout === cliVersion; |
There was a problem hiding this comment.
Could stdout include a newline? Maybe use .trim() on the string.
There was a problem hiding this comment.
Good point, I have addressed the change.
github-action/src/main.ts
Outdated
| @@ -33,10 +33,18 @@ export async function runMain(): Promise<void> { | |||
| ); | |||
| return; | |||
| } | |||
| const devContainerCliInstalled = await devcontainer.isCliInstalled(exec); | |||
| const specifiedDevContainerCliVersion = | |||
| core.getInput('cliVersion') ?? 'latest'; | |||
There was a problem hiding this comment.
Still use '0' as the default, so we can introduce breaking changes on the CLI by increasing its major version. Please use a constant, so we have it only once.
There was a problem hiding this comment.
I have introduced a constant to be referenced as the fallback version if no version is specified.
| @@ -23,10 +23,18 @@ export async function runMain(): Promise<void> { | |||
| ); | |||
| return; | |||
| } | |||
| const devContainerCliInstalled = await devcontainer.isCliInstalled(exec); | |||
| const specifiedDevContainerCliVersion = | |||
| task.getInput('cliVersion') ?? 'latest'; | |||
There was a problem hiding this comment.
Same as below: Still use '0' as the default, so we can introduce breaking changes on the CLI by increasing its major version. Please use a constant, so we have it only once.
| "cacheFrom": "ghcr.io/devcontainers/ci-devcontainer:latest", | ||
| "args": { | ||
| // This is a temporary workaround when developing from host -> remote host -> devcontainer | ||
| // see: https://github.com/microsoft/vscode-remote-release/issues/7958 |
There was a problem hiding this comment.
It was mentioned that updating Docker helped. Is this still needed?
There was a problem hiding this comment.
I was unable to get the devcontainer to build without this even after updating to the latest docker extension for vscode so I have left this as is.
This is in the context of devcontainers/cli#606.
I think this should be enough to allow for CLI version specification according to the other code I saw while reading through this.
The local build in the devcontainer is passing.