sandboxes: follow-up fixes and clarifications from sbx docs rewrite#24591
sandboxes: follow-up fixes and clarifications from sbx docs rewrite#24591dvdksn merged 7 commits intodocker:mainfrom
Conversation
The link to the Monitoring section in policy.md used the wrong anchor (#monitoring-network-activity). The actual heading generates #monitoring. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Step 2 (merged) previously stopped immediately with "no further action needed", skipping all comment handling. Now it checks for unanswered review comments and replies to them. Also adds GraphQL thread-resolution to step 5 so threads get marked resolved after being addressed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- troubleshooting: add mkdir -p ./result prerequisite before tar extract - docker-desktop: replace vague "limited features" with direct pointer to sbx CLI - docker-desktop: clarify restart (not source) is what matters for daemon env vars - docker-desktop: promote Windows Docker daemon caveat to NOTE callout - get-started: replace "elevated permissions" with explicit "Run as Administrator" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ktop page The docker sandbox (legacy) command doesn't have the Docker-not-included limitation on Windows — that only applies to the sbx CLI. Remove the incorrect NOTE callout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Make clear no local Docker daemon is needed - Disambiguate "They" — replace with "These variants" - Note that privileged mode applies inside the microVM boundary, not the host Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/review |
Restarting Docker Desktop is what causes the daemon to pick up the new env var. Opening a new terminal already gives the user the variable. Sourcing explicitly adds nothing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reverts the unnecessary change to the sourcing step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
This follow-up PR correctly addresses the bot review feedback from #24590. All the fixes are appropriate:
✅ Anchor link fixed: #monitoring-network-activity → #monitoring in troubleshooting.md
✅ Prerequisites clarified: Added "Run as Administrator" instruction for Windows Hypervisor Platform setup
✅ Missing step added: Added mkdir -p ./result prerequisite in troubleshooting example
✅ Privileged mode clarified: Specified that -docker variants run in privileged mode "inside the microVM (not on your host)"
✅ Shell sourcing improved: Clarified that sourcing is optional for current terminal, required action is restarting Docker Desktop
✅ Windows limitation corrected: Removed incorrect statement about Docker daemon not being available in docker sandbox command
The changes follow the style guide, improve clarity, and don't introduce any new issues. Nice work addressing the feedback systematically!
Summary
Follow-up to #24590 (sbx docs rewrite):
Learnings
Generated by Claude Code