Repository navigation
chore(make): add docker-skip convenience target to Makefile.terraform - #1519
Conversation
cristim
added
triaged
|
@coderabbitai review |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthrough
ChangesDocker-skip deployment
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
cristim
changed the title
Adversarial review summaryReviewed the single-file Makefile.terraform diff line by line, cross-checked every claim in the diff's comment/PR body against the actual terraform tree, and dry-ran the new target. Verified correct (no bugs found):
One confirmed finding, fixed:
Not fixed / out of scope:
No code changes were required - the diff itself is correct and minimal-impact. Only the PR title was adjusted. @coderabbitai review was already rate-limited (25 min cooldown, see CodeRabbit's comment above). A recovery |
|
Tip For best results, initiate chat on the files or code changes.
The retitled PR now accurately describes the shipped change. For the pre-existing Terraform documentation drift, would you like me to open a separate follow-up issue? ✏️ Learnings added
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 31 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 1 minute. |
|
CodeRabbit has not yet produced a substantive review of this PR: all prior bot comments on it are review-limit / auto-pause notices, so the absence of findings is "not reviewed" rather than "reviewed clean". Requesting a full pass now that the limit window has elapsed. @coderabbitai full review |
|
🐇🔎 ✅ Action performedFull review finished. |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile.terraform`:
- Line 139: Update the hard-coded help output in the Makefile’s help target to
include a Docker operations entry for the docker-skip target, using its existing
“Deploy without building the Docker image” description. Keep the docker-skip
target definition unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
Addressed the The help output is hard-coded at lines 13-39 and does not consume the Worth stating why this was more than a doc nit: the target exists purely as a convenience wrapper around Verified @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes. |
A stashed WIP had docker-build/docker-skip/frontend-skip targets wrapping skip_docker_build/skip_docker_push/enable_frontend_build terraform vars. Of those, only the "skip the whole docker build" case still maps to a real variable (enable_docker_build, which replaced skip_docker_build). skip_docker_push and enable_frontend_build no longer exist anywhere in terraform/environments/*/variables.tf, so docker-build (build without push) and frontend-skip have no current equivalent and are documented as such instead of reintroduced.
Addresses the CodeRabbit finding on Makefile.terraform:139. The help output is hard-coded at lines 13-39 and does not consume the `##` target descriptions, so docker-skip was undiscoverable through `make help` despite carrying one. That matters more than a normal doc gap here: the target exists purely as a convenience wrapper, so a convenience nobody can find is not one. Verified `make -f Makefile.terraform help` now lists it and `make -n -f Makefile.terraform docker-skip` still expands unchanged.
36d3ddb to
47994ec
Compare
|
Rebased onto
No content change from the rebase, just a new base. @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 2 minutes. |
|
Requesting a full review (not incremental) now that a window may be open. Triggers are serialised centrally across the queue because CodeRabbit is under an account-level adaptive throttle and parallel pings hold it in backoff. @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 42 minutes. |
|
Requesting a full review (not incremental) now that a window may be open. Triggers are serialised centrally across the queue because CodeRabbit is under an account-level adaptive throttle and parallel pings hold it in backoff. @coderabbitai full review |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 27 minutes. |
1 participant
Summary
docker-build(skip_docker_push),docker-skip(skip_docker_build),frontend-skip(enable_frontend_build).skip_docker_pushandenable_frontend_buildno longer exist anywhere interraform/environments/*/variables.tf(they only survive in stalemain.tf.bakfiles). The build module has no build-without-push option, and frontend assets are now bundled into the app/Docker build rather than gated by a separate Terraform flag, sodocker-buildandfrontend-skiphave no current equivalent.docker-skipstill maps to a real variable:enable_docker_build(which replaced the oldskip_docker_build). Added that target, adapted to the current wrapper style (scripts/tf-deploy.sh), and left a comment documenting why the other two were not resurrected.Test plan
make -n -f Makefile.terraform docker-skipdry-runs and shows the expectedtf-deploy.sh ... -var="enable_docker_build=false"commandmake -n -f Makefile.terraform frontend-onlyand default target still dry-run cleanly (no syntax breakage)enable_docker_buildis declared interraform/environments/{aws,azure,gcp}/variables.tfskip_docker_push/enable_frontend_builddo not appear in any active (non-.bak) terraform file on mainSummary by CodeRabbit