fix: guard sync target and fix .ruler path separator - #114
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 16 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Guards the sync target to avoid collisions when the Makefile is included by parent repositories, and fixes the .ruler source path so rsync works reliably in CI.
Changes:
- Wrap
synctarget in aDOTAGENTS_SKIP_SYNCconditional. - Fix
.rulerpath construction by adding the missing/afterabspath.
Comments suppressed due to low confidence (1)
Makefile:1
ifeq ($(DOTAGENTS_SKIP_SYNC),)works, butifndef DOTAGENTS_SKIP_SYNCis clearer/idiomatic for “run only when unset” and avoids subtle behavior if the variable is set to whitespace. Consider switching toifndef(optional) to make the intent more immediately readable.
.DEFAULT_GOAL := help
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @make skills-sync | ||
| @make mcp-sync | ||
| @make ruler-dotdirs-sync | ||
| endif |
There was a problem hiding this comment.
ifeq ($(DOTAGENTS_SKIP_SYNC),) works, but ifndef DOTAGENTS_SKIP_SYNC is clearer/idiomatic for “run only when unset” and avoids subtle behavior if the variable is set to whitespace. Consider switching to ifndef (optional) to make the intent more immediately readable.
| @make skills-sync | ||
| @make mcp-sync | ||
| @make ruler-dotdirs-sync |
There was a problem hiding this comment.
These recursive invocations should use $(MAKE) instead of make so flags (e.g., -j jobserver, --no-print-directory, SHELL, etc.) propagate correctly. Suggested change: replace each @make ... with @$(MAKE) ....
Summary
synctarget withDOTAGENTS_SKIP_SYNCconditional to prevent recipe collision when Makefile is included from parent repos via-include dotagents/Makefile/separator inruler-apply-globalpath:$(abspath ...).ruler->$(abspath ...)/.rulerThe
abspathfunction strips trailing slashes, so.rulerwas concatenated directly to the directory name (e.g.dotagents.rulerinstead ofdotagents/.ruler), causing rsync to fail in CI.Closes shunkakinoki/dotfiles#1372
Summary by cubic
Guarded the
syncMakefile target withDOTAGENTS_SKIP_SYNCand fixed the.rulerpath inruler-apply-globalto stop CIrsyncfailures. Prevents target collisions when included from parent repos and fixes the missing path separator.syncwith aDOTAGENTS_SKIP_SYNCcheck to avoid recipe collisions when-include dotagents/Makefileis used.$(abspath ...)/.rulerinstead of$(abspath ...).ruler;abspathstrips trailing slashes, which produceddotagents.rulerand brokersyncin CI.Written for commit 5b78a00. Summary will update on new commits.