Conversation
- Fix apps/testapp/Dockerfile: copy go.mod/go.sum before source so the go mod download layer is stable across code-only changes - Add cache-from/cache-to GHA cache to docker-build-push.yml with per-app scopes to prevent cache eviction between parallel builds - Add cache-dependency-path: "**/go.sum" to all actions/setup-go steps in test.yml, docker-tests.yml, and lint.yml so the module cache key covers all go.sum files in the multi-module repo - Add explicit scope to the e2e Docker build cache in test.yml to align with the docker-build-push.yml scope naming Closes #3196 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @tac0turtle's task in 1m 19s —— View job Code Review
This is a clean, focused CI optimization PR. The changes are well-scoped and the implementation is correct. Here's my detailed review:
|
📝 WalkthroughWalkthroughAdds persistent Buildx/GHA Docker layer caching and scopes cache entries; configures Go module caching via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3213 +/- ##
=======================================
Coverage 61.41% 61.41%
=======================================
Files 120 120
Lines 12474 12474
=======================================
Hits 7661 7661
Misses 3953 3953
Partials 860 860
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/testapp/Dockerfile`:
- Around line 20-23: The Dockerfile currently only primes the root module cache
(COPY go.mod go.sum ./ and RUN go mod download) and uses a banned cd pattern
(RUN cd apps/testapp && go install .); update the build stage to COPY the
testapp module files (apps/testapp/go.mod and apps/testapp/go.sum) into the
image before running dependency download, switch to using WORKDIR to enter
apps/testapp instead of the cd pattern, and run go mod download / go install
from that WORKDIR (replace RUN cd apps/testapp && go install . with a WORKDIR
apps/testapp and RUN go install ./... or equivalent) so cache invalidation is
correct and hadolint DL3003/DL3062 are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bffcc61d-516e-4b29-8a76-25441fe5dcca
📒 Files selected for processing (5)
.github/workflows/docker-build-push.yml.github/workflows/docker-tests.yml.github/workflows/lint.yml.github/workflows/test.ymlapps/testapp/Dockerfile
…nings Also copy apps/testapp/go.mod and apps/testapp/go.sum before running go mod download so the testapp's own dependencies are cached in their own layer (separate from the root module). Replace RUN cd ... && go install with WORKDIR + go build to resolve hadolint DL3003 and DL3062. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/testapp/Dockerfile (1)
24-24:⚠️ Potential issue | 🟠 MajorUse
WORKDIRinstead ofcdin the dependency download step.Line 24 still uses
cdinsideRUN, which triggers hadolint DL3003 and currently fails lint CI. Move directory switching toWORKDIRand run the secondgo mod downloadthere.Suggested patch
COPY go.mod go.sum ./ COPY apps/testapp/go.mod apps/testapp/go.sum ./apps/testapp/ -RUN go mod download && (cd apps/testapp && go mod download) +RUN go mod download +WORKDIR /ev-node/apps/testapp +RUN go mod download # Copy the rest of the source and build. +WORKDIR /ev-node COPY . . WORKDIR /ev-node/apps/testapp RUN go build -o /go/bin/testapp .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/testapp/Dockerfile` at line 24, The RUN line uses "cd" which triggers hadolint DL3003; replace the inline "cd apps/testapp && go mod download" by switching the build context to that directory with a WORKDIR instruction (e.g., set WORKDIR to the app directory before running the second go mod download) and then run "go mod download" there; update the Dockerfile so the first go mod download runs in the repo root, add WORKDIR apps/testapp, run go mod download, and if needed restore the previous WORKDIR afterward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/testapp/Dockerfile`:
- Line 24: The RUN line uses "cd" which triggers hadolint DL3003; replace the
inline "cd apps/testapp && go mod download" by switching the build context to
that directory with a WORKDIR instruction (e.g., set WORKDIR to the app
directory before running the second go mod download) and then run "go mod
download" there; update the Dockerfile so the first go mod download runs in the
repo root, add WORKDIR apps/testapp, run go mod download, and if needed restore
the previous WORKDIR afterward.
1 participant
Summary
apps/testapp/Dockerfile: was copying all source beforego mod download, invalidating the download layer on every code change. Now copiesgo.mod/go.sumfirst, downloads deps, then copies source — same pattern already used by theevmDockerfile.docker-build-push.ymlviacache-from/cache-to: type=ghawith per-app scopes so the 3 parallel image builds don't evict each other's cache.cache-dependency-path: "**/go.sum"to allactions/setup-gosteps intest.yml,docker-tests.yml, andlint.yml. This repo has multiplego.modfiles; the glob ensures the Go module cache key covers all of them.test.yml(scope=ev-node-testapp) to align with thedocker-build-push.ymlnaming and avoid cross-job cache conflicts.Closes #3196
Test plan
go.mod/go.sumfiles changedsetup-gosteps (check "Restore cache" step output in Actions logs)🤖 Generated with Claude Code
Summary by CodeRabbit