-
Notifications
You must be signed in to change notification settings - Fork 56
feat(devcontainers-cli): add devcontainers-cli module #425
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
Conversation
devcontainers-cli/README.md
Outdated
```tf | ||
module "devcontainers-cli" { | ||
source = "registry.coder.com/modules/devcontainers-cli/coder" | ||
version = "release/claude-code/1.0.32" |
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.
Does release/claude-code/1.0.32
exist or is it an 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.
This one is enforced by the update-version.sh
script - not sure if @matifali has more info here on why.
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 should probably be release/devcontainers-cli/1.0.0
.
It might be nice to add Docker detection here as well and print a WARNING if |
Thanks @mafredri and @mtojek for the review - I added :
A question asked by Marcin about About naming, I'm up to keep |
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.
Minor suggestions but otherwise looking good, thanks for implementing support for multiple package managers! 🎉
devcontainers-cli/README.md
Outdated
```tf | ||
module "devcontainers-cli" { | ||
source = "registry.coder.com/modules/devcontainers-cli/coder" | ||
version = "release/claude-code/1.0.32" |
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 should probably be release/devcontainers-cli/1.0.0
.
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.
👍
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.
Some minor optional adjustments to improve the output, but nothing blocking on my side. Nice work!
Co-authored-by: Mathias Fredriksson <[email protected]>
Co-authored-by: Mathias Fredriksson <[email protected]>
PR Is ready, currently waiting for #426 to have easier versioning. |
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.
All good for me with @matifali last changes ✅ Thanks. |
Related to this issue
Create a new module to install @devcontainers/cli using npm.