Repository navigation
Update the git hooks: the fixes of the rollout review - #376
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Findings
|
| Layer / File(s) | Summary |
|---|---|
Pre-commit staged-change routing .githooks/pre-commit, .githooks/pre-commit.jl |
The launcher checks staged .jl and test/ paths before requiring Julia. The Julia script routes staged files and deletions to applicable checks and rejects staged .jl paths that are not files. |
Pre-commit checks and process bounds .githooks/pre-commit.jl |
Formatter checks run per file. Project.toml is parsed with TOML. Package loading and child processes use timeouts and process-group cleanup. |
Pre-push ref gating and project metadata .githooks/pre-push, .githooks/pre-push.jl |
The launcher buffers all ref lines and runs the Julia script only when a remote ref targets main or master. The script reads the project name with TOML and reports parse errors. |
Pre-push test process handling .githooks/pre-push.jl |
On interruption, the test suite’s process group receives SIGINT, followed by SIGKILL after the grace period if it remains active. The failure message names both main and master. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Possibly related PRs
- JuliaGNI/GeometricMachineLearning.jl#375: Introduced the Julia-based Git hooks and launcher structure that this PR updates.
Merge Risk: 
Moderate · up to e8ad3
-
An interrupted push can leave a test child running beyond the promised cleanup period. Fix the process-group check before merging.
Pre-merge checks: 
5
-
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
There was a problem hiding this comment.
Actionable comments posted: 1

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d386e61c-3dc7-4414-a0a6-a637a3b2ead6
Files selected for processing: 4-
.githooks/pre-commit.githooks/pre-commit.jl.githooks/pre-push.githooks/pre-push.jl
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
-
| wait(p) | ||
| signal(Base.SIGINT) | ||
| killer = Timer(_ -> signal(Base.SIGKILL), grace) | ||
| while process_running(p) |
There was a problem hiding this comment.
Keep the kill timer active while the process group survives.
If the Pkg.test process exits after SIGINT but its test child remains, process_running(p) becomes false. The hook then closes the timer without sending SIGKILL to that child. The child can outlive the hook, which defeats the new 30-second cleanup limit. Check the process group before closing the timer, rather than checking only p. Julia documents process_running(p) as a check on the specified process. (docs.julialang.org)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #376 +/- ##
=======================================
Coverage 94.48% 94.48%
=======================================
Files 63 63
Lines 2738 2738
=======================================
Hits 2587 2587
Misses 151 151 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The git hooks get the fixes that the review of the first rollout found.
.jlpath that is not a file, such as a symlink to a missing file or to a directory, is refused with one line instead of a stack trace..jlfile runs the load test.TOML.parsefile, so an indented or single-quotednamecounts, and aProject.tomlthat does not parse is refused.juliawhether a check must run, so ajuliathat cannot start no longer blocks a commit with no.jlfile or a push of a topic branch.fatouholds the output.mainandmaster.The files are byte-identical to
githooks/(andscripts/test-layout.jl) of the ResearchHarness at commita578570, asharness githooks --applyinstalls them. Onlypre-commitandpre-pushare executable.Nothing else changes. After the merge,
harness githooksreports nothing to do for this repository.🤖 Generated with Claude Code