Skip to content

Commit 2a1c593

Browse files
committed
fix: address review feedback on .envrc/lefthook/Makefile cleanup
- .envrc:23 missed the Go-version bump message; align with REQUIRED_VERSION. - .envrc:30 numeric awk comparison treated 1.9 as >= 1.26; split major/minor and compare as integers. - .envrc:60 git rev-parse can fail (no git, not in a repo) and set -e would abort the whole .envrc; guard the call and skip the hooks check cleanly. - lefthook.yml:174 had the same `.git/hooks/pre-commit` hardcoded path that fails in worktrees; mirror the .envrc fix. - Makefile: drop the snap/securecodewarrior/gosec install hints (snap pkg doesn't exist; the repo URL is 404) and use `go install github.com/securego/gosec/v2/cmd/gosec@latest` consistently. - Makefile:300 still hardcoded "60.1%" coverage; replace with a pointer to `make test-coverage` so it can't drift. Addresses gemini-code-assist and Copilot review comments on PR #598. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
1 parent 1100080 commit 2a1c593

3 files changed

Lines changed: 34 additions & 20 deletions

File tree

.envrc

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,23 @@ fi
1919
echo -e "${BLUE}🔧 Verifying Ofelia development environment...${NC}"
2020

2121
# Check Go version requirement
22+
REQUIRED_VERSION="1.26"
23+
2224
if ! command -v go &> /dev/null; then
23-
echo -e "${RED}❌ Go not found. Please install Go 1.25+${NC}"
25+
echo -e "${RED}❌ Go not found. Please install Go ${REQUIRED_VERSION}+${NC}"
2426
exit 1
2527
fi
2628

2729
GO_VERSION=$(go version | grep -o 'go[0-9]\+\.[0-9]\+' | sed 's/go//')
28-
REQUIRED_VERSION="1.26"
2930

30-
if echo "$GO_VERSION $REQUIRED_VERSION" | awk '{exit ($1 < $2)}'; then
31+
# Compare major.minor as integers, not floats — naive numeric comparison
32+
# would incorrectly treat 1.9 as greater than 1.26.
33+
go_major=${GO_VERSION%%.*}
34+
go_minor=${GO_VERSION#*.}
35+
req_major=${REQUIRED_VERSION%%.*}
36+
req_minor=${REQUIRED_VERSION#*.}
37+
38+
if (( go_major > req_major )) || { (( go_major == req_major )) && (( go_minor >= req_minor )); }; then
3139
echo -e "${GREEN}✅ Go $GO_VERSION detected${NC}"
3240
else
3341
echo -e "${YELLOW}⚠️ Go $GO_VERSION found, but Go $REQUIRED_VERSION+ is recommended for this project${NC}"
@@ -56,11 +64,19 @@ check_tool "docker" "https://docs.docker.com/get-docker/"
5664
echo ""
5765
echo -e "${BLUE}🪝 Verifying Git hooks setup...${NC}"
5866

59-
# Resolve via git rev-parse so this works in worktrees (.git is a file, not a dir)
60-
HOOK_FILE="$(git rev-parse --git-path hooks 2>/dev/null)/pre-commit"
67+
# Resolve via git rev-parse so this works in worktrees (.git is a file, not a dir).
68+
# Tolerate failure (no git installed, direnv loaded outside a repo) so set -e
69+
# doesn't abort the entire .envrc — just skip the hooks check.
70+
HOOKS_DIR=""
71+
if command -v git >/dev/null 2>&1; then
72+
HOOKS_DIR="$(git rev-parse --git-path hooks 2>/dev/null || true)"
73+
fi
74+
HOOK_FILE="${HOOKS_DIR:+$HOOKS_DIR/pre-commit}"
6175

62-
if [[ -f "$HOOK_FILE" ]] && grep -q "lefthook" "$HOOK_FILE" 2>/dev/null; then
76+
if [[ -n "$HOOK_FILE" ]] && [[ -f "$HOOK_FILE" ]] && grep -q "lefthook" "$HOOK_FILE" 2>/dev/null; then
6377
echo -e "${GREEN}✅ Git hooks configured (lefthook)${NC}"
78+
elif [[ -z "$HOOK_FILE" ]]; then
79+
echo -e "${YELLOW}⚠️ Skipping hooks check (not in a git repository)${NC}"
6480
else
6581
echo -e "${RED}❌ CRITICAL: Git hooks not configured!${NC}"
6682
echo -e "${YELLOW} Without pre-commit hooks, your commits WILL FAIL the CI pipeline${NC}"

Makefile

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,8 @@ security-check:
9696
echo "✅ Security check passed"; \
9797
else \
9898
echo "❌ gosec not found. Install with:"; \
99-
echo " - Snap: sudo snap install gosec"; \
100-
echo " - Binary: https://github.com/securecodewarrior/gosec/releases"; \
101-
echo " - Or run: make dev-setup"; \
99+
echo " go install github.com/securego/gosec/v2/cmd/gosec@latest"; \
100+
echo " Or run: make dev-setup"; \
102101
exit 1; \
103102
fi
104103

@@ -214,14 +213,9 @@ dev-setup:
214213
@if command -v gosec >/dev/null 2>&1; then \
215214
echo "✅ gosec already available"; \
216215
else \
217-
echo "📥 Installing gosec via snap..."; \
218-
if command -v snap >/dev/null 2>&1; then \
219-
sudo snap install gosec; \
220-
else \
221-
echo "⚠️ gosec not found and snap unavailable. Install manually:"; \
222-
echo " - Snap: sudo snap install gosec"; \
223-
echo " - Binary: https://github.com/securecodewarrior/gosec/releases"; \
224-
fi; \
216+
echo "📥 Installing gosec..."; \
217+
go install github.com/securego/gosec/v2/cmd/gosec@latest; \
218+
echo "✅ gosec installed"; \
225219
fi
226220
@go install github.com/daixiang0/gci@latest
227221
@echo "✅ gci installed"
@@ -297,7 +291,7 @@ help:
297291
@echo " ci - Run CI checks locally"
298292
@echo " tidy - Tidy Go modules"
299293
@echo ""
300-
@echo "📊 Current Test Coverage: 60.1%"
294+
@echo "📊 Test Coverage: run 'make test-coverage' for current numbers"
301295
@echo "🎯 Quality: 45+ linting rules, security scanning, pre-commit hooks"
302296

303297
build:

lefthook.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,12 @@ post-checkout:
170170
echo ""
171171
fi
172172
173-
# Check if hooks are installed (in case this is first checkout)
174-
if [ ! -f .git/hooks/pre-commit ] || ! grep -q "lefthook" .git/hooks/pre-commit 2>/dev/null; then
173+
# Check if hooks are installed (in case this is first checkout).
174+
# Use git rev-parse so this works in worktrees, where .git is a file
175+
# pointing at the worktree's gitdir rather than a directory.
176+
hooks_dir=$(git rev-parse --git-path hooks 2>/dev/null || true)
177+
hook_file="${hooks_dir:+$hooks_dir/pre-commit}"
178+
if [ -z "$hook_file" ] || [ ! -f "$hook_file" ] || ! grep -q "lefthook" "$hook_file" 2>/dev/null; then
175179
echo ""
176180
echo "⚠️ Git hooks not installed! Run:"
177181
echo " make setup"

0 commit comments

Comments
 (0)