-
Notifications
You must be signed in to change notification settings - Fork 22
[Git] CC-1745: Upgrade Bun to v1.2, Rust to v1.86 #151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ly in configuration files and documentation
WalkthroughThis update increases the required versions of Rust (from 1.85 to 1.86) and Bun (from 1.1 to 1.2) across configuration files, documentation, and Dockerfiles for both starter templates and solutions. New Dockerfiles for the updated versions are introduced, and all relevant instructions and version comments are aligned accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dockerfile
participant BuildScript
User->>Dockerfile: Build container (Rust 1.86 or Bun 1.2)
Dockerfile->>Dockerfile: Set environment variables
Dockerfile->>Dockerfile: Copy project files (exclude .git, README.md)
alt Rust
Dockerfile->>BuildScript: Run .codecrafters/compile.sh
else Bun
Dockerfile->>Dockerfile: Run bun install --frozen-lockfile
Dockerfile->>Dockerfile: Cache node_modules if present
end
Dockerfile-->>User: Ready container with updated toolchain
Possibly related PRs
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
dockerfiles/bun-1.2.Dockerfile (1)
16-18
: Consolidate RUN instructions to reduce layers
You can merge the directory creation and conditional move into a singleRUN
to optimize image layers:- RUN mkdir -p /app-cached - RUN if [ -d "/app/node_modules" ]; then mv /app/node_modules /app-cached; fi + RUN mkdir -p /app-cached \ + && if [ -d "/app/node_modules" ]; then mv /app/node_modules /app-cached; fidockerfiles/rust-1.86.Dockerfile (3)
9-10
: 🏗️ Suggest using.dockerignore
instead of--exclude
flags
Hadolint flagsCOPY --exclude
as DL1000, and the canonical Docker way to omit files is via a.dockerignore
. This also simplifies theCOPY
step:-# hadolint ignore=DL1000 -COPY --exclude=.git --exclude=README.md . /app +COPY . /appAnd in your repository root, add a
.dockerignore
with:.git README.md
This removes the need for the experimental
--exclude
flags and silences the linter.🧰 Tools
🪛 Hadolint (2.12.0)
[error] 10-10: invalid flag: --exclude
(DL1000)
12-13
: 🔧 Ensure compile script is executable and portable
If.codecrafters/compile.sh
lacks a shebang or execute bit, the build may fail. Consider either:
- Prefixing the command:
RUN sh ./.codecrafters/compile.sh
- Ensuring the file has a shebang (
#!/usr/bin/env bash
) and execute permissions in Git.This will make your CI builds more robust.
1-13
: 📌 Optional: Pin the Rust image to a digest for reproducibility
To guarantee identical builds over time, consider pinning the base image to its immutable digest:FROM rust@sha256:<digest>-bookworm
You can find the digest via
docker buildx imagetools inspect rust:1.86-bookworm
.🧰 Tools
🪛 Hadolint (2.12.0)
[error] 10-10: invalid flag: --exclude
(DL1000)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
compiled_starters/rust/README.md
(1 hunks)compiled_starters/rust/codecrafters.yml
(1 hunks)compiled_starters/typescript/README.md
(1 hunks)compiled_starters/typescript/codecrafters.yml
(1 hunks)dockerfiles/bun-1.2.Dockerfile
(1 hunks)dockerfiles/rust-1.86.Dockerfile
(1 hunks)solutions/rust/01-gg4/code/README.md
(1 hunks)solutions/rust/01-gg4/code/codecrafters.yml
(1 hunks)solutions/typescript/01-gg4/code/README.md
(1 hunks)solutions/typescript/01-gg4/code/codecrafters.yml
(1 hunks)starter_templates/rust/config.yml
(1 hunks)starter_templates/typescript/config.yml
(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
dockerfiles/rust-1.86.Dockerfile
[error] 10-10: invalid flag: --exclude
(DL1000)
dockerfiles/bun-1.2.Dockerfile
[error] 9-9: invalid flag: --exclude
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test_course_definition / test (haskell)
🔇 Additional comments (15)
solutions/typescript/01-gg4/code/README.md (1)
30-30
: Bump Bun version in README
The update frombun (1.1)
tobun (1.2)
is correct and aligns with the PR objectives.compiled_starters/typescript/README.md (1)
30-30
: Update local Bun requirement
Changing the required Bun version to1.2
is consistent with the companion configuration and Dockerfiles.starter_templates/typescript/config.yml (1)
2-2
: Upgraderequired_executable
to Bun 1.2
The bump frombun (1.1)
tobun (1.2)
matches the other TypeScript starter and solution configs.solutions/typescript/01-gg4/code/codecrafters.yml (1)
10-11
: Align language pack to Bun 1.2
Updating both the comment andlanguage_pack
value tobun-1.2
is correct and consistent across the project.compiled_starters/typescript/codecrafters.yml (1)
10-11
: Switch compiled starter to Bun 1.2
The available versions comment andlanguage_pack
have been properly updated tobun-1.2
.dockerfiles/bun-1.2.Dockerfile (3)
1-2
: Base image version bump is correct
The newoven/bun:1.2-alpine
image aligns with the PR objective to upgrade Bun to v1.2.
4-4
: Environment variable for dependency file paths looks good
SettingCODECRAFTERS_DEPENDENCY_FILE_PATHS="package.json,bun.lockb"
correctly informs the build tooling which files to watch.
11-14
: Lockfile installation for reproducible builds is correct
Usingbun install --frozen-lockfile
ensures CI fails ifpackage.json
andbun.lockb
drift, which is ideal for locked dependencies.starter_templates/rust/config.yml (1)
2-2
: Bump required cargo version to 1.86
Therequired_executable
entry has been correctly updated from 1.85 to 1.86, matching the PR objective.compiled_starters/rust/README.md (1)
30-30
: Update local cargo requirement in README
The Stage 2 instructions now requirecargo (1.86)
, aligning with the updated starter configuration.solutions/rust/01-gg4/code/README.md (1)
30-30
: Align solution README with Rust 1.86
The README has been updated to requirecargo (1.86)
, consistent with the starter and compiled-starter changes.solutions/rust/01-gg4/code/codecrafters.yml (1)
10-11
: Upgrade language_pack to rust-1.86
The# Available versions
comment andlanguage_pack
field are now set torust-1.86
, reflecting the bump.compiled_starters/rust/codecrafters.yml (1)
10-11
: Upgrade compiled starter to Rust 1.86
Both the comment andlanguage_pack: rust-1.86
have been correctly updated to support Rust 1.86.dockerfiles/rust-1.86.Dockerfile (2)
1-3
: ✅ Bumped Rust base image appropriately
The switch torust:1.86-bookworm
aligns perfectly with the PR objective to upgrade Rust from 1.85 to 1.86. The# syntax=docker/dockerfile:1.7-labs
directive correctly enables BuildKit features required by the later instructions.
5-6
: ✅ Dependency-trigger ENV var set correctly
DefiningCODECRAFTERS_DEPENDENCY_FILE_PATHS="Cargo.toml,Cargo.lock"
will ensure cache invalidation when those files change. Confirm that your.codecrafters/compile.sh
script reads this variable as expected.
WORKDIR /app | ||
|
||
# .git & README.md are unique per-repository. We ignore them on first copy to prevent cache misses | ||
COPY --exclude=.git --exclude=README.md . /app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid COPY flag: --exclude
is not supported
Docker’s COPY
instruction does not accept --exclude
flags (Hadolint DL1000). To exclude .git
and README.md
, add them to a .dockerignore
:
.git
README.md
Then update the Dockerfile:
- COPY --exclude=.git --exclude=README.md . /app
+ COPY . /app
🧰 Tools
🪛 Hadolint (2.12.0)
[error] 9-9: invalid flag: --exclude
(DL1000)
Summary by CodeRabbit
New Features
Chores