-
Notifications
You must be signed in to change notification settings - Fork 53
Update OCaml Dockerfile to install dependencies with tests #294
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
WalkthroughThe Dockerfile for OCaml 5.2.0 has been updated. The changes include an addition of the "test/dune" path to the environment variable Changes
Sequence Diagram(s)sequenceDiagram
participant Dockerfile as DF
participant Container as C
participant Opam as O
DF->>C: Build with updated dependency paths
C->>O: Run opam install with "--deps-only" & "--with-test"
O-->>C: Install package and test dependencies
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 0
🧹 Nitpick comments (1)
dockerfiles/ocaml-5.2.0.Dockerfile (1)
29-29
: Enhanced Dependency Installation CommandThe updated
RUN opam install . --yes --deps-only --with-test
command now limits the installation to dependencies (including test-related ones) without installing the package itself. This change aligns with the PR objective of installing dependencies with tests. Please verify that including test dependencies at build time is intentional and compatible with your overall image/build strategy (for example, ensuring it doesn’t unintentionally bloat a production image).Consider adding an inline comment in the Dockerfile to explain the rationale behind these flags for future maintainers.
17fbbd4
to
f0b5328
Compare
Pull Request is not mergeable
Thanks @underwoo16 for highlighting the issue! |
A previous version was already merged. Closing this one. |
Update OCaml Dockerfile to install dependencies with tests
Context:
https://secure.helpscout.net/conversation/2888884607/7893?viewId=8414074
Summary by CodeRabbit