-
Notifications
You must be signed in to change notification settings - Fork 260
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
Features/definitions collection proposal. #15
Conversation
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.
A few first pass comments:
| type | string | singleContainer/orchestrated | | ||
| categories | string | The categories where it belongs | | ||
| architectures | string | Supported architectures | | ||
| baseOs | string | The operating system in which the definition is based | |
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.
Is the ideae here that this might dictate what features could be applied?
The old definition-manifest.json "baseOS" property was used more for handling defaulting in the CI system (and OSS registration - which is not a general capability). If we want to make this more of an option that drives behaviors, I guess the question is what we would put here.
How specific would we want to be? e.g. possible examples:
linux
debian
bullseye
If debian
or bullseye
, how do we handle cases where more than one is supported? (e.g. most pre-built images offer multiple OS versions, some like C++ offer both Ubuntu and Debian)
I could see shifting this more to an "provided version codename" property that is an array for example.
Would we not also want to add the same thing to features then?
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.
I'm leaning more towards keeping everything in keywords and (like you mention in one of the other comments) keeping a list of recommended and recognized keywords that developers can use. And even a list of recommended 'types' of keywords. I removed 'baseOs' and 'architectures' following this idea.
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.
Do we want to instead document known tags/keywords? e.g. certainly whether arm64 is support will be of great interest to macOS users. I could see filtering on what works on the chip architecture right off the bat. A list of architecture may be one that does warrant its own property otherwise.
| architectures | string | Supported architectures | | ||
| baseOs | string | The operating system in which the definition is based | | ||
| options | array | List of user selected options at the moment the user selects them in a client application. | | ||
| styles | array | Variations of the generated definition that contain different information. (e.g. with example code or data.) | |
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.
Isn't this what we call "variants" today? Why styles? Right now this just drives the Dockerfile, bit in concept it could be broader which this seems to allude to. Or is this intended to be more of a "type"?
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.
I think we'll want to stick to the current naming convention for features
, so an options
object like this:
"options": {
"VARIANT": {
"type": "string",
"proposals": [
"1",
"1.16",
"1.17",
"1-bullseye",
"1.16-bullseye",
"1.17-bullseye",
"1-buster",
"1.16-buster",
"1.17-buster"
],
"default": "1-bullseye"
},
"NODE_VERSION": {
"type": "string",
"proposals": [
"none",
"lts/*",
"16",
"14",
"12",
"10"
],
"default": "none"
}
}
},
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.
Ah - yeah if this is options, We could, however, enable referencing an image directly in this model as well, where an option just updates the "image" field rather than args. We could have a type of "image" and "buildArg" to distinguish between the two.
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.
Changed to variants, I forgot it already existed. The idea is more of a declaration of different release files that correspond to the same definition. Added some more information.
|
||
| Property | Type | Description | | ||
|:--- |:--- |:--- | | ||
| requires | array | List of features that are required for this feature to work. | |
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.
This could quickly get into a dependency graph where we'd need to start having the equivalent of lockfiles as well. It will also make it hearder for security minded organizations have their own copy of these that are not public, so we'd need to consider that as well. How would someone create their own private set from a public one?
With that in mind, would it make sense to separate this idea into another proposal instead? I don't know that its explicitly required for what is described here.
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.
I've opened issue #16 on this topic/proposal 👍
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.
Removed and just left a TODO
|
||
# Search | ||
|
||
Search would be implemented by client applications but the definitions themselves should include enough information for this to happen. Besides the typed properties defined above keywords could be: |
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 it is worth calling out that some keywords may be defined by clients as having an affect on UX or any "scoring" done for suggestions.
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.
Added that we should create a list of well-known keywords including those defined by clients.
|
||
For this we propose the existence of two different files: | ||
- Collection file: (devcontainer-collection.json) Groups of features/definitions that will be handled as the same origin (repository/publisher). Contains the metadata pertinent for each one, checksum and other information. | ||
- Feature/Definition: (devcontainer-feature.json/devcontainer-definition.json) Information on a particular feature/definition required to add it to a collection. This file is optional and exists to support scenarios where one repository represents a collection and others represent each feature. |
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.
I think we'll want to make a decision here, either:
- Create one top-level "devcontainer-collection.json
file that has
features: [ ]and
definitions: []` arrays storing the metadata. - Each individual feature and definition has their own "snippet" of metadata, that the GitHub action will then stitch together
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.
The way I envisioned it is actually both. Each feature/definition will be a release with a tar file with the source and json file with its description. An action stitches them together into a collection file. This should simplify the work that clients have to do to find and search collections while still allowing users to control where each feature/definition is actually stored and generated.
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.
Got it. For now i've placed all the definitions into the a single file in vscode-dev-container, but could easily stitch that together with the existing GitHub Action
@@ -0,0 +1,110 @@ | |||
# Proposal |
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.
Am I missing the reference the concept of centralized 'index(es)'.
RIght now I have this flat file with a single reference to a single collection: https://github.com/joshspicer/devcontainer-index
We would then add pointers here to collections we "endorse". Others in the community would be able to produce their own indexes, which point whatever set of collections they would like. Additional indexes could be added to Codespaces via org policy, or via vscode by a setting.
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.
I see the indexes as more of a client feature, where clients define how to store and search it. Also, how users would add a reference to a collection to search their internal features for example.
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.
Is that where we would have the 3 collections from Codespaces, VS Code and the community to start with?
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.
Yea, I want us to publish an index file that will point to our three collections. All we need to do is hardcode a single index file, and that will point to the N
collections we want to pull from by default.
As more of a "visual" - this is my understanding of the spec for a subset of the vscode-dev-container definitions. I'm looking to merge in the next day or so as an interactive example to this proposal. |
| repositoryPath | string | Path to the repository and folder that contains the root of the code for the feature/definition. | | ||
| keywords | array | List of keywords relevant to a user that would search for this definition/feature. | | ||
| version | string | Versioning information for the feature/definition. | | ||
| files | array | List of links for the download files with checksums and optional signature information. | |
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.
Individual files? (Not tar packages?)
Will there be a tar per feature or a tar per collection?
| Property | Type | Description | | ||
| :--- | :--- | :--- | | ||
| type | string | singleContainer/orchestrated | | ||
| options | array | List of user selected options at the moment the user selects them in a client application. | |
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.
Like the ones we currently parse out of the Dockerfile?
Variants: | ||
| Property | Type | Description | | ||
| :--- | :--- | :--- | | ||
| name | string | Name of the variant | |
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.
Is each variant a definition by itself? (I guess that would require a link/pointer to that definition?)
|
||
### Definitions | ||
|
||
| Property | Type | Description | |
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.
At a later point we could add an icon (that can be shown in the UI next to the name) and an optional link to a sample.
- Language. TS, JS, C#, Java, etc. | ||
- Other Apps. Databases, simulators, etc. | ||
- Dependencies. Bare metal, Azure, AWS, etc. | ||
- Organization. |
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.
Additional data that we might want to represent as separate properties (not as keywords) because we are using them programmatically (currently parsed from the definition's README.md):
- A flag for 'core definitions' to show only these when we don't have any other way to filter down the list of definitions in the UI. (Not sure how to represent that when we accept anyones definitions, maybe this should be a list on the products' side.)
- Lists of platforms/languages to filter down the list of definitions when we know some of the file types used in the repository. (This can be done as keywords if that's preferred.)
- A flag indicating compatibility with Codespaces: To be product-neutral, we might want to have a list of known compatible products and a list of known incompatible products, so each product can then decide whether to include a definition that does not list the product at all.
@@ -0,0 +1,110 @@ | |||
# Proposal |
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.
Is that where we would have the 3 collections from Codespaces, VS Code and the community to start with?
| version | string | Versioning information for the feature/definition. | | ||
| files | array | List of links for the download files with checksums and optional signature information. | | ||
|
||
### Definitions |
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.
Do we still want to call this 'definitions' :)
|
||
### Features | ||
|
||
TODO: Add properties related to references/dependencies. |
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.
Can this link to the other spec?
As was being discussed here.
I wanted to writeup a more formal proposal about the way it could work to foment discussion.