-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(devcontainer): Migrate to template-based project structure #5
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
📝 WalkthroughWalkthroughMigrates from a single root Dev Container to a template-based project layout: deletes root Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @projects/.template/.devcontainer/devcontainer.json:
- Line 9: Remove the redundant initializeCommand entry in devcontainer.json that
runs "docker volume create ${localEnv:VSCODE_CONTAINER_PROJECT}" because the
named volume is already declared in compose-base/docker-compose.yaml (the same
${localEnv:VSCODE_CONTAINER_PROJECT} volume) and Docker Compose will create it;
delete the "initializeCommand" property (or omit that specific shell command) so
volume creation is handled solely by the docker-compose configuration.
In @projects/.template/docker-compose.yaml:
- Around line 7-9: Remove the unused build arg SDK_VERSION from the
docker-compose service args (delete the line "SDK_VERSION: 0"); it's not
referenced by images/base/Dockerfile (which only has a FROM), so remove the
SDK_VERSION entry to avoid a no-op build argument and keep the compose file
clean.
In @README.md:
- Around line 10-13: Update the README step 4 to include a concrete example
showing how to set the VSCODE_CONTAINER_PROJECT environment variable (e.g.,
export VSCODE_CONTAINER_PROJECT="my-project" for macOS/Linux,
$env:VSCODE_CONTAINER_PROJECT="my-project" for PowerShell, and setx
VSCODE_CONTAINER_PROJECT "my-project" for Windows CMD) and note alternative
placement in a shell profile or a .env file if users prefer; reference the
VSCODE_CONTAINER_PROJECT variable name and place the example immediately after
the existing step 4 so users see exactly how to set it before reopening in the
container.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.devcontainer/devcontainer.json.gitignoreREADME.mdcompose-base/docker-compose.yamlprojects/.template/.devcontainer/devcontainer.jsonprojects/.template/docker-compose.yaml
💤 Files with no reviewable changes (1)
- .devcontainer/devcontainer.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build dotnet Image
🔇 Additional comments (5)
.gitignore (1)
1-2: LGTM! Clean gitignore pattern for template workflow.The pattern correctly ignores generated project directories while preserving the template for version control.
compose-base/docker-compose.yaml (2)
8-8: Ensure VSCODE_CONTAINER_PROJECT is documented as required.The volume mount path uses
${VSCODE_CONTAINER_PROJECT}which must be set in the host environment. Verify that the README and setup documentation clearly communicate this requirement and the consequences if it's not set.
5-6: Configuration is correct; no action needed.The Dockerfile path resolves properly. The base compose defines
context: ..(repo root from compose-base/), and the template specifiesdockerfile: images/base/Dockerfile. Docker Compose resolves dockerfile paths relative to the build context, so the merged configuration correctly points to the existingimages/base/Dockerfileat the repository root.README.md (1)
3-3: LGTM! Clear documentation of the template-based workflow.The README effectively documents the migration to a template-based approach, explains the role of
VSCODE_CONTAINER_PROJECT, and provides clear setup instructions. The explanation of why an environment variable is needed (devcontainer.json limitations) adds valuable context.Also applies to: 15-15, 19-19
projects/.template/.devcontainer/devcontainer.json (1)
10-10: No action needed — Thevscodeuser is provided by themcr.microsoft.com/devcontainers/baseimage, which is Microsoft's official dev container base image that includes this user by default. ThepostCreateCommandwill work as intended.Likely an incorrect or invalid review 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.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @projects/.template/.devcontainer/devcontainer.json:
- Around line 9-10: The devcontainer.json uses "postCreateCommand": "sudo chown
vscode ${containerWorkspaceFolder}" and relies on the vscode user implicitly;
add an explicit "remoteUser": "vscode" property to the devcontainer
configuration so VS Code connects as that user, keeping "postCreateCommand" and
"overrideCommand" intact; update the JSON to include remoteUser so the intended
user is clear and connection behavior is consistent.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
projects/.template/.devcontainer/devcontainer.json
🔇 Additional comments (2)
projects/.template/.devcontainer/devcontainer.json (2)
1-11: LGTM! Clean template-based configuration.The devcontainer configuration is well-structured and correctly implements the template-based approach. The removal of the previously flagged redundant
initializeCommandwas the right decision—Docker Compose will handle volume creation automatically based on the compose file definitions.
2-2: No action needed. TheVSCODE_CONTAINER_PROJECTenvironment variable is already documented in the README. Step 4 of the setup instructions explicitly states to "SetVSCODE_CONTAINER_PROJECTin the host environment," and the Configuration Notes section explains its purpose: "it provides the project identity (volume name and/mnt/<project>path); it is an environment variable becausedevcontainer.jsoncannot pass variables into Docker Compose."
| "postCreateCommand": "sudo chown vscode ${containerWorkspaceFolder}", | ||
| "overrideCommand": true |
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.
🧹 Nitpick | 🔵 Trivial
Consider adding remoteUser property for clarity.
While the postCreateCommand implies the container uses the vscode user, explicitly specifying "remoteUser": "vscode" would make the configuration clearer and ensure VS Code connects as the intended user.
🔎 Proposed enhancement
"postCreateCommand": "sudo chown vscode ${containerWorkspaceFolder}",
- "overrideCommand": true
+ "overrideCommand": true,
+ "remoteUser": "vscode"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "postCreateCommand": "sudo chown vscode ${containerWorkspaceFolder}", | |
| "overrideCommand": true | |
| "postCreateCommand": "sudo chown vscode ${containerWorkspaceFolder}", | |
| "overrideCommand": true, | |
| "remoteUser": "vscode" | |
| } |
🤖 Prompt for AI Agents
In @projects/.template/.devcontainer/devcontainer.json around lines 9 - 10, The
devcontainer.json uses "postCreateCommand": "sudo chown vscode
${containerWorkspaceFolder}" and relies on the vscode user implicitly; add an
explicit "remoteUser": "vscode" property to the devcontainer configuration so VS
Code connects as that user, keeping "postCreateCommand" and "overrideCommand"
intact; update the JSON to include remoteUser so the intended user is clear and
connection behavior is consistent.
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Quick Notes
Title: Set the PR title to
@coderabbitaito have the bot generate one for you.Review: The bot reviews PRs by default. To opt out, add the
no bot reviewerlabel.