Uh oh!
There was an error while loading. Please reload this page.
feat(cli): rewrite set-up command to full devcontainer spec compliance - #251
Conversation
Rewrites the set-up command from a minimal stub (2 lifecycle hooks) to a full spec-compliant BYOC implementation: - All 5 lifecycle hooks in spec order (onCreateCommand, updateContentCommand, postCreateCommand, postStartCommand, postAttachCommand) - Container validation via docker inspect before executing - Merged config pipeline (ParseDevContainerJSON + MergeConfiguration) - remoteEnv + containerEnv support via -e flags - --result-format support for JSON envelope output - --docker-path flag for custom docker/podman executable - Feature installation into running containers (resolve, stage, docker cp, docker exec install scripts) - Export GetFeatureEnvVariables and GetFeatureInstallWrapperScript for reuse in BYOC feature installation path E2E tests extended with onCreateCommand, updateContentCommand, postAttachCommand, remoteEnv, lifecycle ordering, and local feature installation tests.
✅ Deploy Preview for devsydev canceled.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR expands the setup command orchestration by promoting feature utility functions to public API, completely rewriting the setup flow to include container inspection, configuration loading, feature installation, and lifecycle hook execution, alongside comprehensive test coverage and end-to-end validation. ChangesSetup Orchestration & Feature API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
…fallback - stageFeatures now receives the resolved remoteUser from GetRemoteUser(result) instead of hardcoding "root" in devcontainer-features.builtin.env - resolveWorkdir no longer falls back to host-CWD-based path; returns empty string when no workspace folder is configured (buildDockerExecArgs already skips --workdir when empty)
Extract repeated lifecycle hook name strings into package-level constants and repeated test command string into a test constant.
Rename hookOnCreateCommand/hookPostCreateCommand/etc. in setup.go to match the shorter hookOnCreate/hookPostCreate/etc. names already used in the test package, and remove the now-duplicate declarations from runusercommands_test.go. Also use testCmdEcho constant in exec_test.go to avoid repeated "echo" string literals.
"hello" appeared 4 times across exec_test.go and setup_test.go. Extract to a shared testCmdHello constant and use it in both files.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Rewrites the
set-upcommand from a minimal 2-hook stub to full devcontainer spec compliance for BYOC (Bring Your Own Container) workflows:E2E tests extended with onCreateCommand, updateContentCommand, postAttachCommand, remoteEnv, lifecycle ordering, and local feature installation coverage.
Summary by CodeRabbit
New Features
Tests