feat(dotfiles-updater): skip build/switch when no changes detected - #558
Conversation
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces an efficiency improvement to the periodic Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Only run install.sh (make build && make switch) when there are actual git changes from origin/main. This avoids unnecessary rebuilds during the periodic 3hr update cycle.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR adds a sensible optimization to the dotfiles updater, skipping the build and switch when there are no new commits. The implementation is correct and will save resources. One point of feedback for future improvement: the install.sh script, which is subsequently called, performs redundant git fetch and pull operations. Since this update.sh script now ensures the repository is up-to-date, these operations in install.sh could be skipped, for instance by checking the AUTOMATED_UPDATE environment variable. This is out of scope for this PR but would be a good follow-up to improve efficiency.
| REMOTE_COMMIT=$(git rev-parse origin/main) | ||
|
|
||
| # Check if there are any changes | ||
| if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then |
There was a problem hiding this comment.
For better robustness and to follow modern bash best practices, it's recommended to use the [[ ... ]] compound command for tests instead of the POSIX [ ... ]. The [[ keyword is specific to bash (and other modern shells) and is safer as it prevents issues like word splitting on variable expansions. While your use of quotes makes the current code safe, [[ is generally preferred for new bash code. When using [[, it's also idiomatic to use == for string comparison.
| if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then | |
| if [[ "$CURRENT_COMMIT" == "$REMOTE_COMMIT" ]]; then |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a pre-fetch commit comparison to the dotfiles-updater script: it records the current local commit, fetches Changes
Sequence Diagram(s)(omitted — changes are localized to a single script and test suite; not generating diagrams) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{sh,bash}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
🔇 Additional comments (2)
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 |
Add ShellSpec tests to verify: - Current commit is stored before fetch - Remote commit is retrieved after fetch - Commits are compared to detect changes - Appropriate messages are logged for both cases
There was a problem hiding this comment.
Pull request overview
This PR optimizes the periodic dotfiles-updater service by adding a check to skip the expensive build and switch operations when no changes are detected in the remote repository.
Changes:
- Add commit hash comparison before and after fetching to detect changes
- Skip the install.sh execution when current commit matches remote commit
- Add detailed logging to show which commits changed for debugging
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then | ||
| echo "No changes detected (current: ${CURRENT_COMMIT:0:8}). Skipping build and switch." | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The new commit comparison logic lacks test coverage. Consider adding tests to verify:
- The script exits early when CURRENT_COMMIT equals REMOTE_COMMIT
- The script proceeds with installation when commits differ
- The appropriate log messages are output in each case
This is important functionality that should be tested given the repository has comprehensive test coverage for shell scripts.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/services/dotfiles-updater/update.sh">
<violation number="1" location="home-manager/services/dotfiles-updater/update.sh:25">
P2: Early exit skips `git reset --hard`, leaving dirty working tree unchanged when no upstream changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Check if there are any changes | ||
| if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then | ||
| echo "No changes detected (current: ${CURRENT_COMMIT:0:8}). Skipping build and switch." | ||
| exit 0 |
There was a problem hiding this comment.
P2: Early exit skips git reset --hard, leaving dirty working tree unchanged when no upstream changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/dotfiles-updater/update.sh, line 25:
<comment>Early exit skips `git reset --hard`, leaving dirty working tree unchanged when no upstream changes.</comment>
<file context>
@@ -10,8 +10,24 @@ if [ "$(git rev-parse --abbrev-ref HEAD)" != "main" ]; then
+# Check if there are any changes
+if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then
+ echo "No changes detected (current: ${CURRENT_COMMIT:0:8}). Skipping build and switch."
+ exit 0
+fi
+
</file context>
| exit 0 | |
| git reset --hard origin/main | |
| exit 0 |
ShellCheck complains about unexpanded expressions in single quotes, but these are intentional literal string matches in test assertions.
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 2535acb...e42dc40
Analysis
-
No state tracking for failed installs - only checks if git commits match, not whether the last install.sh run was successful. Failed installs won't retry until new commits arrive.
-
Limited error handling strategy - script lacks explicit error handling for git operations. Network failures during git fetch will cause hard exit without clear debugging information.
-
Silent discarding of local changes - git reset --hard operation will remove any local modifications without warning, potentially causing unexpected behavior for users testing changes.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 2 comments | Edit Agent Settings • Read Docs
| REMOTE_COMMIT=$(git rev-parse origin/main) | ||
|
|
||
| # Check if there are any changes | ||
| if [ "$CURRENT_COMMIT" = "$REMOTE_COMMIT" ]; then |
There was a problem hiding this comment.
This optimization skips the install when commits match, but doesn't account for scenarios where the previous install.sh run may have failed partially. Consider tracking the last successful install commit in a state file (e.g., ~/.dotfiles-updater-last-success) and comparing against that instead of just HEAD. This would ensure a retry if the previous install failed.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#558
File: home-manager/services/dotfiles-updater/update.sh#L23
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
This optimization skips the install when commits match, but doesn't account for scenarios where the previous install.sh run may have failed partially. Consider tracking the last successful install commit in a state file (e.g., ~/.dotfiles-updater-last-success) and comparing against that instead of just HEAD. This would ensure a retry if the previous install failed.
| CURRENT_COMMIT=$(git rev-parse HEAD) | ||
|
|
||
| # Fetch latest changes | ||
| git fetch origin main |
There was a problem hiding this comment.
Consider adding error handling for the git fetch operation. If the fetch fails (network issues, remote unavailable), the script will exit due to set -e, but REMOTE_COMMIT will never be set. Adding explicit error handling or at least logging would make debugging easier:
if ! git fetch origin main; then
echo "Failed to fetch from origin/main"
exit 1
fiAgent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#558
File: home-manager/services/dotfiles-updater/update.sh#L17
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Consider adding error handling for the git fetch operation. If the fetch fails (network issues, remote unavailable), the script will exit due to `set -e`, but REMOTE_COMMIT will never be set. Adding explicit error handling or at least logging would make debugging easier:
```bash
if ! git fetch origin main; then
echo "Failed to fetch from origin/main"
exit 1
fi
</details>
Mesa DescriptionTL;DRDotfiles updater now fetches origin/main, compares HEAD to it, and exits early if identical—skipping install.sh (make build && make switch). This makes the 3‑hour update a fast no‑op when there are no changes and saves CPU. What changed?
WhySaves CPU/time when there are no dotfiles changes to apply. The 3hr update cycle will now be a quick no-op check instead of a full rebuild. Summary by cubicDotfiles updater now fetches origin/main, compares HEAD to it, and exits early if identical—skipping install.sh (make build && make switch). This makes the 3‑hour update a fast no‑op when there are no changes and saves CPU. Written for commit e42dc40. Summary will update on new commits. Description generated by Mesa. Update settings |
Mesa DescriptionTL;DRDotfiles updater now fetches origin/main, compares HEAD to it, and exits early if identical—skipping install.sh (make build && make switch). This makes the 3‑hour update a fast no‑op when there are no changes and saves CPU. What changed?
Description generated by Mesa. Update settings |
Mesa DescriptionTL;DRSkip the What changed?
Description generated by Mesa. Update settings |
Summary
Skip the `make build && make switch` step when the periodic 3hr dotfiles-updater runs and there are no new changes in git.
Changes
Why
Saves CPU/time when there are no dotfiles changes to apply. The 3hr update cycle will now be a quick no-op check instead of a full rebuild.
Summary by cubic
Dotfiles updater now fetches origin/main, compares HEAD to it, and exits early if identical—skipping install.sh (make build && make switch). This makes the 3‑hour update a fast no‑op when there are no changes and saves CPU.
Written for commit b4dd1d8. Summary will update on new commits.