-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Kafka] [CC-1642] Upgrade Gleam to 1.9 #26
Conversation
Update Gleam starter files to use Gleam 1.9.1, including: - Updating codecrafters.yml to use gleam-1.9 - Modifying README to specify gleam (1.9.1) - Adding Dockerfile for Gleam 1.9 - Updating starter template configuration
WalkthroughThe changes update the Gleam version requirements and associated configuration settings across multiple files. Specifically, README files now require Gleam version 1.9.1 rather than a broad 1.0+ range, and configuration files update the language pack from gleam-1.4 to gleam-1.9. Additionally, a new Dockerfile for Gleam 1.9.1 has been added with build caching and dependency management commands. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant DS as Docker System
participant Gleam as Gleam Build Process
Dev->>DS: Trigger build using dockerfiles/gleam-1.9.Dockerfile
DS->>DS: Set base image (v1.9.1) and environment variables
DS->>DS: Copy files (excluding .git and README.md)
DS->>Gleam: Execute `gleam build` to fetch dependencies
DS->>DS: Cache build output in /app-cached
DS-->>Dev: Return built Docker image
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (1)
dockerfiles/gleam-1.9.Dockerfile (1)
15-17
: Optimizing Build Layer UsageThe commands to create the caching directory and move the build output are correct. To optimize the Docker image by reducing the number of layers, consider combining these two commands into a single RUN instruction.
A suggested diff:
-# Cache build directory -RUN mkdir -p /app-cached -RUN mv build /app-cached/build +# Cache build directory in a single layer +RUN mkdir -p /app-cached && mv build /app-cached/build
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
compiled_starters/gleam/README.md
(1 hunks)compiled_starters/gleam/codecrafters.yml
(1 hunks)dockerfiles/gleam-1.9.Dockerfile
(1 hunks)solutions/gleam/01-vi6/code/README.md
(1 hunks)solutions/gleam/01-vi6/code/codecrafters.yml
(1 hunks)starter_templates/gleam/config.yml
(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
dockerfiles/gleam-1.9.Dockerfile
[error] 10-10: invalid flag: --exclude
(DL1000)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: test_course_definition / test (rust)
- GitHub Check: test_course_definition / test (python)
- GitHub Check: test_course_definition / test (javascript)
- GitHub Check: test_course_definition / test (java)
- GitHub Check: test_course_definition / test (go)
- GitHub Check: test_course_definition / test (gleam)
- GitHub Check: test_course_definition / test (csharp)
- GitHub Check: test_course_definition / test (cpp)
🔇 Additional comments (9)
solutions/gleam/01-vi6/code/codecrafters.yml (1)
10-11
: Updated Language Pack VersionThe language pack has been updated to use
gleam-1.9
, which aligns with the new dependency requirements. Please verify that this version is supported by all related tooling.compiled_starters/gleam/codecrafters.yml (1)
10-11
: Consistent Update to Language PackThe update to
language_pack: gleam-1.9
ensures consistency across your configuration files. This clearly reflects the intended upgrade.solutions/gleam/01-vi6/code/README.md (1)
30-30
: Revised Required Gleam Version in DocumentationThe README now instructs users to have
gleam (1.9.1)
installed locally instead of a general version range. This update is critical in setting the correct expectations.compiled_starters/gleam/README.md (1)
30-30
: Updated Version Requirement in User InstructionsThe change to require
gleam (1.9.1)
maintains documentation consistency with the other configuration files and ensures users install the correct version.starter_templates/gleam/config.yml (1)
2-2
: Refined Executable RequirementThe
required_executable
is now updated togleam (1.9.1)
, which provides a precise version requirement and aligns with the documentation changes.dockerfiles/gleam-1.9.Dockerfile (4)
1-2
: Base Image and Docker Syntax Declaration are CorrectThe Dockerfile correctly declares the Docker syntax version and uses the intended base image for Gleam v1.9.1.
4-5
: Environment Variable Setup for Dependency FilesThe comment and environment variable are clear in intent; they signal cache invalidation when either
gleam.toml
ormanifest.toml
changes.
7-7
: Working Directory DeclarationSetting the working directory to
/app
is straightforward and appropriate for this build context.
12-13
: Dependency Installation StepThe
RUN gleam build
command is used to force dependency downloads. This step is clear and aligns with the intended behavior for ensuring that all dependencies are present.
# .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.
🛠️ Refactor suggestion
Review COPY Command Flags
The COPY command uses --exclude
flags which are not officially supported by Docker’s COPY instruction, as confirmed by the Hadolint warning. Instead, consider using a .dockerignore
file to exclude .git
and README.md
from the context.
A potential refactor diff for this change is:
-COPY --exclude=.git --exclude=README.md . /app
+COPY . /app
Ensure you add a .dockerignore
file in the repository root with the following entries:
.git
README.md
🧰 Tools
🪛 Hadolint (2.12.0)
[error] 10-10: invalid flag: --exclude
(DL1000)
Summary by CodeRabbit