docs: use mounts for examples#91
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughREADME example commands for sandbox creation across all agent/provider flows (Anthropic, OpenCode+Anthropic, Ollama, OpenAI, Vertex AI) are updated to use ChangesREADME Sandbox Example Updates
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
509-538: 💤 Low valueClarify presentation consistency: show both old and new approaches in all examples, or only new.
The first two examples (Claude + Anthropic at 509–538 and OpenCode + Anthropic at 542–571) show both the old
--upload .approach and the new--driver-config-jsonmount approach in a single code block, separated by a comment. However, the OpenAI example (617–636) appears to replace--upload .inline (line 632 marked as changed) without showing both approaches side-by-side for comparison.For consistency and to help users migrate, either:
- Show both old and new in all examples (as in Claude/OpenCode + Anthropic), or
- Simplify all to show only the new approach (removing the old code entirely)
The current mixed presentation may confuse readers about which style the documentation prefers.
Also applies to: 542-571, 617-636
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 509 - 538, The README documentation shows inconsistent presentation of migration approaches across examples. Some examples (Claude + Anthropic and OpenCode + Anthropic) display both the old `--upload .` approach and the new `--driver-config-json` mount approach side-by-side in a single code block separated by comments, while the OpenAI example shows only the new approach inline. Standardize the presentation by choosing either to show both old and new approaches in all examples separated by clarifying comments (to help users migrate), or simplify all examples to show only the new approach (removing the old code entirely) and apply this chosen style consistently across all provider examples in the documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 530: The NVIDIA documentation link on lines 530 and 563 in README.md
contains an invalid anchor fragment `#podman-driver-config-mounts` that does not
exist on the NVIDIA documentation page. Fix this by either verifying and
correcting the anchor to point to the actual section name on the NVIDIA docs
page, or remove the anchor fragment entirely to link to the base documentation
URL. Apply the same correction to both locations where this invalid link
appears.
---
Nitpick comments:
In `@README.md`:
- Around line 509-538: The README documentation shows inconsistent presentation
of migration approaches across examples. Some examples (Claude + Anthropic and
OpenCode + Anthropic) display both the old `--upload .` approach and the new
`--driver-config-json` mount approach side-by-side in a single code block
separated by comments, while the OpenAI example shows only the new approach
inline. Standardize the presentation by choosing either to show both old and new
approaches in all examples separated by clarifying comments (to help users
migrate), or simplify all examples to show only the new approach (removing the
old code entirely) and apply this chosen style consistently across all provider
examples in the documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
9c21865 to
c1d542b
Compare
| $ openshell sandbox create \ | ||
| --from sandbox_image:claude_vertexai \ | ||
| --upload . \ | ||
| --env CLAUDE_CODE_USE_VERTEX=1 \ |
There was a problem hiding this comment.
shouldn't it create first a google-vertex-ai type provider ?
https://docs.nvidia.com/openshell/providers/google-vertex-ai
and then use --provider to give this provider
There was a problem hiding this comment.
I would prefer to wait for this to stabilize on openshell (NVIDIA/OpenShell#1752 and NVIDIA/OpenShell#1763 are still open) before to document it
benoitf
left a comment
There was a problem hiding this comment.
LGTM for the mounting process but I think for vertex ai, it should use now its provider
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Update examples in README to use podman mount feature.