Skip to content

Conversation

@ryzen-xp
Copy link
Contributor

@ryzen-xp ryzen-xp commented Sep 7, 2025

Summary by CodeRabbit

  • Chores
    • Introduced a Docker-based Dev Container for a reproducible VS Code development environment, with automatic installation of required tooling for consistent builds and tests.
  • Documentation
    • Added a DevContainer setup guide, including prerequisites, steps to open the project in a container, and verification commands to build, test, and format the project.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 7, 2025

Walkthrough

Adds a Dev Container setup: a Debian-based .devcontainer.json enabling Rust feature, installing VS Code extensions, and running a postCreateCommand to execute .devcontainer/install-tools.sh. The script installs prerequisites, asdf, and pins versions of scarb, dojo, and starknet-foundry. README gets a new DevContainer usage section.

Changes

Cohort / File(s) Summary
DevContainer Configuration
./.devcontainer.json
Introduces a Debian-based devcontainer with Rust feature (stable), VS Code extensions, postCreateCommand to run install script, and sets remoteUser to vscode.
Dev Tools Install Script
./.devcontainer/install-tools.sh
Adds setup script: installs apt packages, installs/configures asdf, adds plugins (scarb 2.10.1, dojo 1.5.0, starknet-foundry 0.35.0) with idempotent guards, sets globals.
Documentation
./README.md
Adds “DevContainer Setup (Docker)” section with prerequisites, steps, and verification commands.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant VS as VS Code
    participant DC as Dev Container
    participant SH as install-tools.sh
    participant AS as asdf
    Dev->>VS: Open repo
    VS->>DC: Reopen in Container
    DC->>DC: postCreateCommand
    DC->>SH: bash .devcontainer/install-tools.sh
    SH->>DC: apt-get install curl, git, build-essential
    SH->>AS: Install asdf (v0.14.0) & source
    SH->>AS: asdf plugin add scarb/dojo/starknet-foundry
    AS-->>SH: Plugins ready
    SH->>AS: asdf install + set global versions
    AS-->>SH: Tools installed
    SH-->>DC: Setup complete
    DC-->>VS: Environment ready
    VS-->>Dev: Run verification commands
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

In a Debian burrow I brew my dev tea,
VS Code hums softly, containers set free.
Asdf sprinkles plugins—click, clack, cheer!
Scarb, Dojo, Foundry all nestle near.
With paws on the keys, I thump with delight—
“Sozo build!” echoes into the night. 🐇🚀

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (5)
README.md (1)

95-116: Add quick version checks to Verify section

Including explicit version checks helps confirm the toolchain installed correctly inside the DevContainer.

 ### Verify
 Inside the container, run:
 
 ```bash
 sozo build
 sozo test
 scarb fmt --check
+scarb --version
+sozo --version
+snforge --version

</blockquote></details>
<details>
<summary>.devcontainer/install-tools.sh (1)</summary><blockquote>

`30-30`: **Print installed versions to aid debugging**

Small quality-of-life improvement at the end of setup.


```diff
-echo "✅ Environment ready!"
+echo "✅ Environment ready!"
+scarb --version || true
+sozo --version || true
+snforge --version || true
.devcontainer.json (3)

3-3: Pin base image to a specific Debian series for reproducibility

Using a moving “debian” tag can lead to silent changes. Pin to bookworm (or a digest) to stabilize builds.

-  "image": "mcr.microsoft.com/devcontainers/base:debian",
+  "image": "mcr.microsoft.com/devcontainers/base:bookworm",

12-17: Ensure login shell env is picked up (asdf shims on new terminals)

Devcontainers usually source .bashrc, but being explicit avoids PATH surprises.

   },
-  "features": {
+  "features": {
     "ghcr.io/devcontainers/features/rust:1": {
       "version": "stable"
     }
   },
+  "userEnvProbe": "loginInteractiveShell",

20-20: Speed up rebuilds by caching tool dirs

Persist asdf between rebuilds to avoid re-downloading toolchains.

   "remoteUser": "vscode"
+  ,"mounts": [
+    "type=volume,source=devc-asdf,target=/home/vscode/.asdf"
+  ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca5bdeb and 09331f7.

📒 Files selected for processing (3)
  • .devcontainer.json (1 hunks)
  • .devcontainer/install-tools.sh (1 hunks)
  • README.md (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
.devcontainer/install-tools.sh

[warning] 14-14: ShellCheck can't follow non-constant source. Use a directive to specify location.

(SC1090)

⏰ 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-and-test
🔇 Additional comments (2)
.devcontainer/install-tools.sh (1)

14-14: ShellCheck SC1090

You can silence SC1090 for sourcing asdf since the path is user-dependent (already addressed in the diff with a directive).

.devcontainer.json (1)

18-19: postCreate runs as 'vscode'; ensure script uses sudo

Given remoteUser is “vscode”, postCreateCommand must either call sudo for apt or you switch to root for postCreate. Since we fixed the script to use sudo, no change is required here; leaving this note for clarity.

Run the container and confirm postCreate succeeds without permission errors.

@@ -0,0 +1,30 @@
#!/usr/bin/env bash
set -e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden shell options

Prefer strict mode to surface errors early.

-set -e
+set -euo pipefail
+IFS=$'\n\t'
🤖 Prompt for AI Agents
In .devcontainer/install-tools.sh around line 2, the script currently uses only
"set -e" which is not strict enough; replace it with a stricter shell mode by
changing to "set -euo pipefail" and add a safe IFS like "IFS=$'\n\t'" directly
after that (and ensure this is placed near the top of the script, after any
shebang) so undefined variables, pipeline failures, and word-splitting are
handled robustly.

echo "🚀 Setting up development environment..."

# Install dependencies
apt-get update && apt-get install -y curl git build-essential
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Apt commands will fail under non-root remoteUser

postCreateCommand runs as the “vscode” user; calling apt-get without sudo will fail. Use sudo and noninteractive flags.

-apt-get update && apt-get install -y curl git build-essential
+sudo apt-get update -y
+sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends curl git build-essential

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
.devcontainer/install-tools.sh around line 7: the apt commands run as the
non-root "vscode" user in postCreateCommand and will fail; update the line to
call sudo for apt-get update and sudo with a noninteractive frontend and flags
for install (e.g., DEBIAN_FRONTEND=noninteractive and -y/--no-install-recommends
or -qq) so the install runs successfully without prompting and with elevated
privileges.

Comment on lines +11 to +15
git clone https://github.com/asdf-vm/asdf.git ~/.asdf --branch v0.14.0
echo '. "$HOME/.asdf/asdf.sh"' >> ~/.bashrc
echo '. "$HOME/.asdf/completions/asdf.bash"' >> ~/.bashrc
. ~/.asdf/asdf.sh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Make asdf install idempotent and avoid .bashrc duplication

Guard clone, avoid appending duplicate lines, and quiet ShellCheck for sourcing.

-# Install asdf
-git clone https://github.com/asdf-vm/asdf.git ~/.asdf --branch v0.14.0
-echo '. "$HOME/.asdf/asdf.sh"' >> ~/.bashrc
-echo '. "$HOME/.asdf/completions/asdf.bash"' >> ~/.bashrc
-. ~/.asdf/asdf.sh
+# Install asdf (idempotent)
+if [ ! -d "$HOME/.asdf" ]; then
+  git clone https://github.com/asdf-vm/asdf.git "$HOME/.asdf" --branch v0.14.0
+fi
+grep -qxF '. "$HOME/.asdf/asdf.sh"' "$HOME/.bashrc" || echo '. "$HOME/.asdf/asdf.sh"' >> "$HOME/.bashrc"
+grep -qxF '. "$HOME/.asdf/completions/asdf.bash"' "$HOME/.bashrc" || echo '. "$HOME/.asdf/completions/asdf.bash"' >> "$HOME/.bashrc"
+# shellcheck disable=SC1090
+. "$HOME/.asdf/asdf.sh"
📝 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.

Suggested change
git clone https://github.com/asdf-vm/asdf.git ~/.asdf --branch v0.14.0
echo '. "$HOME/.asdf/asdf.sh"' >> ~/.bashrc
echo '. "$HOME/.asdf/completions/asdf.bash"' >> ~/.bashrc
. ~/.asdf/asdf.sh
# Install asdf (idempotent)
if [ ! -d "$HOME/.asdf" ]; then
git clone https://github.com/asdf-vm/asdf.git "$HOME/.asdf" --branch v0.14.0
fi
grep -qxF '. "$HOME/.asdf/asdf.sh"' "$HOME/.bashrc" || echo '. "$HOME/.asdf/asdf.sh"' >> "$HOME/.bashrc"
grep -qxF '. "$HOME/.asdf/completions/asdf.bash"' "$HOME/.bashrc" || echo '. "$HOME/.asdf/completions/asdf.bash"' >> "$HOME/.bashrc"
# shellcheck disable=SC1090
. "$HOME/.asdf/asdf.sh"
🧰 Tools
🪛 Shellcheck (0.10.0)

[warning] 14-14: ShellCheck can't follow non-constant source. Use a directive to specify location.

(SC1090)

🤖 Prompt for AI Agents
In .devcontainer/install-tools.sh around lines 11–15, make the asdf setup
idempotent by only cloning if ~/.asdf doesn't already exist, avoid appending
duplicate lines to ~/.bashrc by checking for each line before adding it, and
silence ShellCheck warnings for sourcing by adding a shellcheck disable comment
just before the source line; implement these checks (e.g., test -d ~/.asdf to
skip git clone, use grep -Fxq to conditionally append each echo line) and add a
"# shellcheck disable=SC1090,SC1091" comment immediately above the ".
~/.asdf/asdf.sh" source invocation.

Comment on lines +17 to +27
asdf plugin add scarb || true
asdf install scarb 2.10.1
asdf global scarb 2.10.1

asdf plugin add dojo https://github.com/dojoengine/asdf-dojo || true
asdf install dojo 1.5.0
asdf global dojo 1.5.0

asdf plugin add starknet-foundry || true
asdf install starknet-foundry 0.35.0
asdf global starknet-foundry 0.35.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid masking real failures with || true; check/add plugins explicitly

Fail fast if a plugin is unavailable, and keep installs DRY with variables.

-# Add and install plugins
-asdf plugin add scarb || true
-asdf install scarb 2.10.1
-asdf global scarb 2.10.1
-
-asdf plugin add dojo https://github.com/dojoengine/asdf-dojo || true
-asdf install dojo 1.5.0
-asdf global dojo 1.5.0
-
-asdf plugin add starknet-foundry || true
-asdf install starknet-foundry 0.35.0
-asdf global starknet-foundry 0.35.0
+# Add and install plugins
+SCARB_VERSION=2.10.1
+DOJO_VERSION=1.5.0
+SNF_VERSION=0.35.0
+
+ensure_plugin() {
+  local name="$1"; shift
+  if ! asdf plugin list | grep -qx "$name"; then
+    asdf plugin add "$name" "$@"
+  fi
+}
+
+ensure_plugin scarb
+asdf install scarb "${SCARB_VERSION}"
+asdf global scarb "${SCARB_VERSION}"
+
+ensure_plugin dojo https://github.com/dojoengine/asdf-dojo
+asdf install dojo "${DOJO_VERSION}"
+asdf global dojo "${DOJO_VERSION}"
+
+ensure_plugin starknet-foundry
+asdf install starknet-foundry "${SNF_VERSION}"
+asdf global starknet-foundry "${SNF_VERSION}"
+
+asdf reshim
📝 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.

Suggested change
asdf plugin add scarb || true
asdf install scarb 2.10.1
asdf global scarb 2.10.1
asdf plugin add dojo https://github.com/dojoengine/asdf-dojo || true
asdf install dojo 1.5.0
asdf global dojo 1.5.0
asdf plugin add starknet-foundry || true
asdf install starknet-foundry 0.35.0
asdf global starknet-foundry 0.35.0
# Add and install plugins
SCARB_VERSION=2.10.1
DOJO_VERSION=1.5.0
SNF_VERSION=0.35.0
ensure_plugin() {
local name="$1"; shift
if ! asdf plugin list | grep -qx "$name"; then
asdf plugin add "$name" "$@"
fi
}
ensure_plugin scarb
asdf install scarb "${SCARB_VERSION}"
asdf global scarb "${SCARB_VERSION}"
ensure_plugin dojo https://github.com/dojoengine/asdf-dojo
asdf install dojo "${DOJO_VERSION}"
asdf global dojo "${DOJO_VERSION}"
ensure_plugin starknet-foundry
asdf install starknet-foundry "${SNF_VERSION}"
asdf global starknet-foundry "${SNF_VERSION}"
asdf reshim
🤖 Prompt for AI Agents
.devcontainer/install-tools.sh lines 17-27: the script masks plugin-add failures
with "|| true", repeats installs, and hardcodes versions; change to explicitly
check for and add each plugin (e.g., use "asdf plugin-list" to detect presence
and run "asdf plugin add" only when missing), remove "|| true" so failures
surface and cause exit, capture and reuse version numbers in variables to avoid
repetition, and run "asdf install" then "asdf global" for each tool while
checking their exit codes and exiting fast on error.

@KevinMB0220 KevinMB0220 merged commit ac2dcc2 into SunsetLabs-Game:main Sep 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants