Skip to content

Conversation

@keivenchang
Copy link
Contributor

@keivenchang keivenchang commented Oct 28, 2025

Overview:

Updates container documentation to reflect workspace path changes from /home/ubuntu/dynamo to /workspace and clarifies -it flag usage for interactive sessions.

Details:

  • Changed container paths from /home/ubuntu/dynamo to /workspace
  • Updated build target paths from .build/target to target
  • Clarified -it flag requirement for interactive terminal sessions

Where should the reviewer start?

.devcontainer/README.md and container/README.md

/coderabbit profile chill

Summary by CodeRabbit

  • Documentation
    • Standardized development container path references throughout documentation for consistency
    • Enhanced guidance on container execution modes: interactive for development sessions and non-interactive for CI/automation workflows

@keivenchang keivenchang self-assigned this Oct 28, 2025
@keivenchang keivenchang requested review from a team as code owners October 28, 2025 00:50
@github-actions github-actions bot added the docs label Oct 28, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 28, 2025

Walkthrough

Both README files received documentation updates: .devcontainer/README.md standardized all container path references from /home/ubuntu/dynamo to /workspace, while container/README.md clarified interactive session requirements with -it flags and adjusted CI automation examples.

Changes

Cohort / File(s) Summary
Dev Container Configuration
.devcontainer/README.md
Updated path references throughout documentation from /home/ubuntu/dynamo to /workspace. Changes applied consistently to mount points, cargo build commands, build artifacts, documentation access paths, and development workflow steps.
Container Usage Documentation
container/README.md
Expanded command examples to clarify interactive versus automated execution modes. Added -it flag guidance for interactive sessions and adjusted CI/automation commands to omit -it for non-interactive contexts. Added explicit "Interactive Mode" documentation to run.sh features.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

These are pure documentation updates with no code logic changes. The alterations consist of straightforward path replacements and clarification of usage patterns with command flags. The changes are homogeneous in nature (consistent pattern application) and low in complexity:

  • Path reference updates are mechanical and repetitive
  • Flag additions follow a consistent pattern across examples
  • No structural or logical reasoning required beyond verifying documentation accuracy

Human review should focus on:

  • Verifying all path references were consistently updated and remain accurate
  • Confirming the -it flag guidance aligns with actual container runtime requirements
  • Ensuring examples accurately reflect both interactive and CI/automation scenarios

Poem

🐰 twitches whiskers with glee

From /dynamo paths we did flee,
To /workspace where we roam free!
Add -it flags, don't you see—
For interactive shells, we agree!
Hop, hop, hop—documentation spree! 🎉

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "docs: update container paths and interactive mode usage in READMEs" accurately and fully summarizes the main changes in the changeset. The title clearly identifies both primary updates: the container path changes from /home/ubuntu/dynamo to /workspace and the clarification of the -it flag for interactive sessions. The title is specific, concise, and directly related to the actual changes in the two README files, making it clear to teammates scanning the history what the main purpose of this PR is.
Description Check ✅ Passed The pull request description is mostly complete and follows the required template structure with all key sections present: Overview clearly states the purpose of the changes, Details provides specific bullet points about what was modified, and Where should the reviewer start identifies the two primary files to review. The description is comprehensive and directly aligned with the changes reflected in the raw summary. The Related Issues section is not included, but this appears to be a documentation update without associated issue references, so this omission does not significantly detract from the overall quality and completeness of the description.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a79122c and c7548ca.

📒 Files selected for processing (2)
  • .devcontainer/README.md (5 hunks)
  • container/README.md (4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-15T02:01:01.238Z
Learnt from: keivenchang
PR: ai-dynamo/dynamo#2453
File: deploy/dynamo_check.py:739-741
Timestamp: 2025-08-15T02:01:01.238Z
Learning: In the dynamo project, missing cargo should be treated as a fatal error in dynamo_check.py because developers need cargo to build the Rust components. The tool is designed to validate the complete development environment, not just import functionality.

Applied to files:

  • .devcontainer/README.md
📚 Learning: 2025-08-30T20:43:49.632Z
Learnt from: keivenchang
PR: ai-dynamo/dynamo#2797
File: container/Dockerfile:437-449
Timestamp: 2025-08-30T20:43:49.632Z
Learning: In the dynamo project's devcontainer setup, the team prioritizes consistency across framework-specific Dockerfiles (like container/Dockerfile, container/Dockerfile.vllm, etc.) by mirroring their structure, even when individual optimizations might be possible, to maintain uniformity in the development environment setup.

Applied to files:

  • .devcontainer/README.md
📚 Learning: 2025-08-30T20:43:10.091Z
Learnt from: keivenchang
PR: ai-dynamo/dynamo#2797
File: .devcontainer/devcontainer.json:12-12
Timestamp: 2025-08-30T20:43:10.091Z
Learning: In the dynamo project, devcontainer.json files use templated container names (like "dynamo-vllm-devcontainer") that are automatically processed by the copy_devcontainer.sh script to generate framework-specific configurations with unique names, preventing container name collisions.

Applied to files:

  • .devcontainer/README.md
📚 Learning: 2025-08-30T20:43:10.091Z
Learnt from: keivenchang
PR: ai-dynamo/dynamo#2797
File: .devcontainer/devcontainer.json:12-12
Timestamp: 2025-08-30T20:43:10.091Z
Learning: In the dynamo project's devcontainer setup, hard-coded container names in devcontainer.json files serve as templates that are automatically processed by the copy_devcontainer.sh script to generate framework-specific configurations with unique names, preventing container name collisions.

Applied to files:

  • .devcontainer/README.md
⏰ 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). (7)
  • GitHub Check: operator (arm64)
  • GitHub Check: vllm (amd64)
  • GitHub Check: trtllm (amd64)
  • GitHub Check: trtllm (arm64)
  • GitHub Check: vllm (arm64)
  • GitHub Check: sglang
  • GitHub Check: Build and Test - dynamo
🔇 Additional comments (2)
container/README.md (1)

116-116: Clear and well-integrated interactive mode documentation.

The updates effectively distinguish interactive (with -it) vs. non-interactive/CI (without -it) usage across multiple examples. The new "Interactive Mode" bullet on line 211 and contextual comments (e.g., line 258) make the requirement explicit.

Also applies to: 128-128, 211-211, 216-220, 258-259, 350-350

.devcontainer/README.md (1)

214-214: Path updates correctly reflect /workspace workspace mount.

All workspace references consistently use /workspace as the container mount point, and build/documentation paths correctly reference /workspace/target. The changes align well with the PR objective to standardize on /workspace paths.

Also applies to: 226-226, 232-232, 247-247, 260-260, 262-262, 271-271


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@keivenchang keivenchang merged commit 12083e9 into main Oct 28, 2025
35 of 43 checks passed
@keivenchang keivenchang deleted the keivenchang/fix-README.md-for-Dev-Container branch October 28, 2025 18:02
csabakecskemeti pushed a commit to csabakecskemeti/dynamo that referenced this pull request Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants