Repository navigation
Add validate-submodules pre-commit hook - #11
Conversation
Co-authored-by: kenorb <kenorb@users.noreply.github.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a local pre-commit hook to validate Git submodule consistency by running File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- As written,
bash -c 'git submodule status'will always exit 0 even if submodules are modified or uninitialized, so if the goal is to enforce consistency you may want a small script that inspects the output (e.g., fails on lines starting with-or+) instead of just running the command directly. - Using
language: systemwithbash -ccouples the hook to Bash being available; consider switching tolanguage: scriptand checking in a small, executable script to make the hook more portable and easier to evolve.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- As written, `bash -c 'git submodule status'` will always exit 0 even if submodules are modified or uninitialized, so if the goal is to enforce consistency you may want a small script that inspects the output (e.g., fails on lines starting with `-` or `+`) instead of just running the command directly.
- Using `language: system` with `bash -c` couples the hook to Bash being available; consider switching to `language: script` and checking in a small, executable script to make the hook more portable and easier to evolve.
## Individual Comments
### Comment 1
<location path=".pre-commit-config.yaml" line_range="101-103" />
<code_context>
- id: detect-secrets
+ - repo: local
+ hooks:
+ - id: validate-submodules
+ name: Validate Submodule Consistency
+ entry: bash -c 'git submodule status'
+ language: system
+ pass_filenames: false
</code_context>
<issue_to_address>
**issue (bug_risk):** Current command likely never fails, so the hook won’t actually enforce consistency.
`git submodule status` exits 0 in most cases and only prints the state, so the hook won’t fail even if submodules are uninitialized or out of date. To actually enforce consistency, use a command that returns non-zero on problems (e.g., `git submodule status --recursive --fail` in Git ≥2.40, or a custom script that checks for unexpected submodule commits).
</issue_to_address>
### Comment 2
<location path=".pre-commit-config.yaml" line_range="103" />
<code_context>
+ hooks:
+ - id: validate-submodules
+ name: Validate Submodule Consistency
+ entry: bash -c 'git submodule status'
+ language: system
+ pass_filenames: false
</code_context>
<issue_to_address>
**suggestion:** Avoid `bash -c` when a direct entry and arguments would work.
For example, configure this as `entry: git` with `args: [submodule, status, --recursive]`. This avoids quoting issues and makes the hook’s behavior clearer and more portable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
/co Address PR comments. |
Co-authored-by: kenorb <kenorb@users.noreply.github.com>
|
I have addressed the PR comments by implementing a more robust submodule validation script and updating the pre-commit configuration. Changes made:
The configuration has been validated using |
|
/co I prefer previous version without external script. Just improve the hook, but without additional script. |
Co-authored-by: kenorb <kenorb@users.noreply.github.com>
|
initialized, modified, or in conflict, and I've removed the Changes:
The configuration has been verified with |


I've added the
validate-submoduleshook to the.pre-commit-config.yamlfile, ensuring it follows the existing lexicographical sorting convention by placing it after the GitHub-hosted repositories. I also verified the configuration's validity usingpre-commit validate-config.Triggered by workflow_dispatch
opencode session | github run