-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes for Features V2 #6
Changes from 26 commits
eebacc5
e825692
1047e01
40aa793
83afa1b
39cff54
12d39a2
e769e66
b35174c
6b59032
1e9a5bb
9b98c33
5923ef7
23e87ff
9bce15f
91edf38
971bd56
4e0fc7e
4842ec5
ebbbc4e
26f3f35
11fb36c
934ae4a
63a5ed2
b304f90
5e29707
ec88721
267ef67
df7575b
1b42a63
68e68b1
39a1b22
e92dd24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
// Use IntelliSense to learn about possible attributes. | ||
// Hover to view descriptions of existing attributes. | ||
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 | ||
"version": "0.2.0", | ||
"configurations": [ | ||
{ | ||
"type": "node", | ||
"request": "launch", | ||
"name": "Launch cli - up", | ||
"program": "${workspaceFolder}/src/spec-node/devContainersSpecCLI.ts", | ||
"cwd": "${workspaceFolder}", | ||
"args": ["up", "--workspace-folder", "../features-playground", "--log-level", "debug", ], | ||
"console": "integratedTerminal", | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,13 +105,18 @@ export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand' | |
|
||
const defaultWaitFor: DevContainerConfigCommand = 'updateContentCommand'; | ||
|
||
export interface DevContainerFeature { | ||
id: string; | ||
options: boolean | string | Record<string, boolean | string | undefined>; | ||
} | ||
|
||
export interface CommonDevContainerConfig { | ||
configFilePath?: URI; | ||
remoteEnv?: Record<string, string | null>; | ||
forwardPorts?: (number | string)[]; | ||
portsAttributes?: Record<string, PortAttributes>; | ||
otherPortsAttributes?: PortAttributes; | ||
features?: Record<string, string | boolean | Record<string, string | boolean>>; | ||
features?: DevContainerFeature[] | Record<string, string | boolean | Record<string, string | boolean>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that we don't want to support the array variant. Please see https://github.com/devcontainers/spec/pull/27/files#r869018039. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Made a comment on that thread. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have shared some thoughts as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dropping my thoughts here since the spec PR has already been merged for a couple weeks. I strongly believe we need to support the array syntax, and I would go a step further and say that we should only document the array syntax going forward, leaving the object syntax in place only for backwards compatibility (object can be easily mapped to the array form at runtime in a deterministic way). There are several scenarios we've encountered from users who need to better control feature execution order as a consumer of the feature in a way that the feature author wouldn't know about in advance. Most of these involve custom logic that the consumer wants to run after a feature is installed, but are things the user wants baked into the image itself and not later on in a lifecycle hook. A few of the examples:
I understand the argument that the tooling should be able to figure out the right order if dependencies are defined, but I don't think we have a solution for dependency declaration within each feature that is robust enough to handle all of the cases. Ultimately, giving users the ability to force a specific ordering puts them in control and allows them to make adjustments as needed instead of being at the mercy of the tooling to figure out the perfect install order. For most users, they won't need to worry about the installation order of different features, and in that case, both syntaxes work just fine. But in the case where users do need the control for the ordering, the array syntax makes that possible, whereas the object syntax does not. |
||
onCreateCommand?: string | string[]; | ||
updateContentCommand?: string | string[]; | ||
postCreateCommand?: string | string[]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we can add in a very simple feature set/collection as one of the tests, and use that here? That way we can always F5 into something useful.