Support gitflow.path.hooks and core.hooksPath for hooks directory#67
Conversation
Adds three-level hooks directory resolution so hook and filter scripts can be located outside the default .git/hooks directory. The precedence order is: gitflow.path.hooks (avh compatibility), core.hooksPath (Git native), then .git/hooks (default). Both absolute and relative paths are supported, with relative paths resolved from the repository root. Refs #63
Adds nine tests covering the three-level hooks directory precedence: default .git/hooks, gitflow.path.hooks (absolute and relative), core.hooksPath (absolute and relative), precedence when both are set, and integration tests verifying hooks and filters actually execute from configured directories. Refs #63
Adds HOOKS DIRECTORY section explaining the three-level precedence for hook script locations. Updates SHARING HOOKS with gitflow.path.hooks and global/system configuration examples. Updates CREATING HOOK SCRIPTS to reference the configurable hooks directory instead of hardcoded path. Refs #63
Adds PATH CONFIGURATION section to gitflow-config manpage documenting the gitflow.path.hooks option, including default behavior, absolute and relative path support, and git-flow-avh compatibility note. Refs #63
There was a problem hiding this comment.
Code Review Summary
This PR successfully implements support for configurable hooks directories via gitflow.path.hooks and core.hooksPath, resolving issue #63. The implementation is clean, well-tested, and follows the project's conventions.
Strengths
-
Excellent three-level precedence logic (lines 48-68 in
internal/hooks/hooks.go):- Clear priority order:
gitflow.path.hooks>core.hooksPath>.git/hooks - Proper avh compatibility with
gitflow.path.hookstaking highest precedence - Clean separation of concerns with
resolveHooksPath()helper
- Clear priority order:
-
Comprehensive test coverage (9 new tests):
- Tests all three precedence levels independently
- Tests both absolute and relative path resolution
- Tests precedence ordering when multiple configs are set
- Tests integration with hooks, post-hooks, and filters
- Clear, descriptive test names and documentation
-
Documentation updates:
- Both
gitflow-config.5.mdandgitflow-hooks.7.mdupdated - Includes practical examples for all configuration methods
- Explains the differences between
gitflow.path.hooksandcore.hooksPath
- Both
-
Backward compatibility:
- Default behavior unchanged (falls back to
.git/hooks) - Preserves worktree support
- No breaking changes to existing functionality
- Default behavior unchanged (falls back to
Areas for Consideration
-
Path resolution edge case (lines 54-55 in hooks.go):
The code assumesrepoRoot = filepath.Dir(commonDir), which works for standard repos where.gitis in the root. For worktrees,commonDirmight point to the main repository's.git, and takingfilepath.Dir()would correctly resolve to the main repo root. This appears correct but could benefit from a comment explaining the worktree case. -
Error handling for non-existent configured directories:
The PR description mentions "Non-existent configured directories cause hooks to be silently skipped, matching Git's own behavior." This is reasonable, but consider whether a warning message would be helpful for users who misconfigure paths. The current implementation at lines 122-129 inrunHook()handles this gracefully by returningHookResult{Executed: false}when the hook path doesn't exist. -
Testability pattern (line 38):
Good use of dependency injection viavar configLookup = git.GetConfigInDirfor testability. However, the tests don't appear to use this mock capability—they use real git config commands instead. This is fine for integration tests, but if you ever need faster unit tests without git operations, this hook is ready.
Minor Suggestions
-
Add a comment in
getHooksDir()explaining the worktree scenario to make the path resolution logic even clearer for future maintainers. -
Consider adding a test for the edge case where
gitflow.path.hooksis set to an empty string explicitly, to verify it falls through tocore.hooksPath. -
Documentation: The examples in both manpage files are excellent and comprehensive.
Recommendation
APPROVE ✅
This is a well-implemented feature with excellent test coverage and documentation. The code is clean, follows Go best practices, and integrates smoothly with existing functionality. The precedence logic is clear and correct, and the avh compatibility is properly maintained.
The minor suggestions above are optional improvements and don't block approval. This PR is ready to merge.
Files reviewed:
internal/hooks/hooks.go✅test/internal/hooks/hooks_test.go✅docs/gitflow-config.5.md✅docs/gitflow-hooks.7.md✅
There was a problem hiding this comment.
Pull request overview
This pull request adds support for configurable hooks directories via gitflow.path.hooks and core.hooksPath configuration settings. This addresses issue #63 by allowing git-flow hooks to be stored outside the .git/hooks/ directory, enabling global or shared hook configurations across multiple projects.
Changes:
- Implements three-level precedence for hooks directory resolution:
gitflow.path.hooks>core.hooksPath>.git/hooks/(default) - Adds support for both absolute and relative paths (resolved from repository root)
- Maintains worktree compatibility with the default fallback
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/hooks/hooks.go | Core implementation of configurable hooks directory resolution with three-level precedence and path resolution logic |
| test/internal/hooks/hooks_test.go | Comprehensive test coverage with 9 new tests covering all resolution paths, precedence, and integration scenarios |
| docs/gitflow-hooks.7.md | Updated documentation explaining the new hooks directory configuration options with examples |
| docs/gitflow-config.5.md | Added PATH CONFIGURATION section documenting the gitflow.path.hooks setting |
Support configurable hooks directories via
gitflow.path.hooksandcore.hooksPath, allowing hooks to live outside.git/hooks/.The
getHooksDir()function now resolves the hooks directory using a three-level precedence:gitflow.path.hooks(git-flow-avh compatible) >core.hooksPath(Git native) >.git/hooks/(default). Both absolute and relative paths are supported, with relative paths resolved from the repository root. This applies to both hooks and filters since they share the same resolution function. Changes touchinternal/hooks/hooks.go,docs/gitflow-hooks.7.md, anddocs/gitflow-config.5.md, with 9 new tests covering all resolution paths and integration scenarios.Resolves #63
Remarks
gitflow.path.hookstakes highest priority for avh compatibility and as an escape hatch whencore.hooksPathis set for Git's own hooks but git-flow hooks should come from elsewhere.git/hooksfallback remains worktree-awareReview focus:
internal/hooks/hooks.go:37-68— three-level precedence logic and path resolutiontest/internal/hooks/hooks_test.go:828-1282— new test coverage