-
Notifications
You must be signed in to change notification settings - Fork 70
chore: Devcontainer updates #4213
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
base: main
Are you sure you want to change the base?
Conversation
…ed by the container host.
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.
Pull Request Overview
This pull request modernizes the devcontainer configuration by simplifying setup and improving compatibility, particularly for ARM-based development environments like Apple Silicon Macs.
- Replaces custom Dockerfile with prebuilt Microsoft devcontainer image
- Adds ARM architecture support for axe linter and other AMD64-dependent tools
- Makes documentation server accessible from outside containers by binding to 0.0.0.0
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.devcontainer/devcontainer.json |
Migrated from custom Dockerfile to prebuilt image, added ARM support via postCreateCommand, updated features to include Node 22.12.0 and jq utilities, moved global npm package installation to onCreateCommand |
.devcontainer/Dockerfile |
Removed in favor of prebuilt image approach |
package.json |
Added --host 0.0.0.0 flag to docu:start script to enable external container access |
.gitignore |
Added .pnpm-store directory to ignore list for cleaner repository |
pomfrida
left a comment
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.
LGTM 👍
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| "docu:start": "pnpm --filter ./apps/design-system-docs run start", | ||
| "docu:build": "pnpm run extract-prerequisites && pnpm --filter ./apps/design-system-docs run build", | ||
| "docu:serve": "pnpm --filter ./apps/design-system-docs run serve", | ||
| "docu:start": "pnpm --filter ./apps/design-system-docs run start --host 0.0.0.0", |
Copilot
AI
Dec 8, 2025
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 docu:serve script is being removed but it's still documented in apps/design-system-docs/README.md (line 43). Either the script should be kept or the documentation should be updated to remove references to it.
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 root README.md refers to versions in the root package.json, not apps/design-system-docs/package.json. The apps/design-system-docs/README.md is specific to its local package.json.
Co-authored-by: Copilot <[email protected]>
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.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| "docu:start": "pnpm --filter ./apps/design-system-docs run start", | ||
| "docu:build": "pnpm run extract-prerequisites && pnpm --filter ./apps/design-system-docs run build", | ||
| "docu:serve": "pnpm --filter ./apps/design-system-docs run serve", | ||
| "docu:start": "pnpm --filter ./apps/design-system-docs run start --host 0.0.0.0", |
Copilot
AI
Dec 8, 2025
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 docu:serve script was removed from package.json but is still referenced in the documentation at apps/design-system-docs/README.md line 43. Since the underlying docusaurus serve command is still available in the app's package.json, consider either:
- Keeping the
docu:servescript wrapper for consistency with otherdocu:*commands, or - Updating the documentation to directly reference
pnpm --filter ./apps/design-system-docs run serve
| "docu:start": "pnpm --filter ./apps/design-system-docs run start --host 0.0.0.0", | |
| "docu:start": "pnpm --filter ./apps/design-system-docs run start --host 0.0.0.0", | |
| "docu:serve": "pnpm --filter ./apps/design-system-docs run serve", |
| "ghcr.io/devcontainers/features/node:1": { | ||
| "version": "22.12.0" | ||
| } |
Copilot
AI
Dec 8, 2025
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 Node.js feature is being added with version 22.12.0, but the base image already specifies typescript-node:22-bullseye which includes Node.js 22. This creates redundancy and potential version conflicts. The Node feature will install a second Node.js version on top of the one already in the base image.
Consider removing the Node feature from the features section since the base image already provides Node.js 22, or use a more minimal base image if you want the features approach. The current setup is redundant and could lead to confusion about which Node.js version is being used.
Copilot summary:
This pull request updates the development container configuration to simplify setup and improve compatibility, especially for ARM-based hosts. The main changes involve switching from a custom Dockerfile to a prebuilt image, updating dependency installation steps, and improving the
docu:startscript for better accessibility.Devcontainer configuration improvements:
.devcontainer/Dockerfilewith a direct reference to the prebuiltmcr.microsoft.com/devcontainers/typescript-node:22-bullseyeimage in.devcontainer/devcontainer.json, removing the need to maintain a separate Dockerfile. [1] [2]postCreateCommandin.devcontainer/devcontainer.jsonto install additional AMD64 libraries for better compatibility with tools like axe linter on ARM-based hosts (e.g., Apple Silicon Macs).featuressection in.devcontainer/devcontainer.jsonto use the latest Node version and add jq-like utilities via a community feature, while removing redundant Docker and SSHD features.pnpm,gitmoji-cli) from the Dockerfile to theonCreateCommandin.devcontainer/devcontainer.jsonfor a more streamlined setup. [1] [2]Script accessibility improvement:
docu:startscript inpackage.jsonto bind the documentation server to0.0.0.0, making it accessible from outside the container.@vnys can you confirm that the features I have removed are in fact redundant?
I have tested build and run of the documentation in both codespace and a local devcontainer on my mac.