feat: add Nit-Bundle type to issue template hook #139

Merged
forgejo_admin merged 1 commit from 137-add-nit-bundle-type-to-check-issue-templ into main 2026-03-21 17:28:03 +00:00
Contributor

Summary

Adds Nit-Bundle as a recognized issue type in the check-issue-template.sh PreToolUse hook, so nit-bundle issues validate against template-issue-nit-bundle instead of falling through to the feature template.

Changes

  • hooks/check-issue-template.sh — Added Nit-Bundle|nit-bundle case to the type-to-template mapping, before the * fallback. Aligned spacing on existing cases for readability.

Test Plan

  • Create an issue with ### Type\nNit-Bundle in the body — hook should fetch template-issue-nit-bundle and validate against it.
  • Create an issue with ### Type\nBug — should still map to template-issue-bug (no regression).
  • Create an issue with no type — should still fall through to template-issue-feature.

Review Checklist

  • Single file changed: hooks/check-issue-template.sh
  • Case statement ordering preserved (specific types before * fallback)
  • Template slug follows existing naming convention (template-issue-{type})
  • No other files modified
## Summary Adds `Nit-Bundle` as a recognized issue type in the `check-issue-template.sh` PreToolUse hook, so nit-bundle issues validate against `template-issue-nit-bundle` instead of falling through to the feature template. ## Changes - `hooks/check-issue-template.sh` — Added `Nit-Bundle|nit-bundle` case to the type-to-template mapping, before the `*` fallback. Aligned spacing on existing cases for readability. ## Test Plan - Create an issue with `### Type\nNit-Bundle` in the body — hook should fetch `template-issue-nit-bundle` and validate against it. - Create an issue with `### Type\nBug` — should still map to `template-issue-bug` (no regression). - Create an issue with no type — should still fall through to `template-issue-feature`. ## Review Checklist - [x] Single file changed: `hooks/check-issue-template.sh` - [x] Case statement ordering preserved (specific types before `*` fallback) - [x] Template slug follows existing naming convention (`template-issue-{type}`) - [x] No other files modified ## Related - Closes #137
The check-issue-template.sh hook now recognizes Nit-Bundle as a valid
issue type, validating against template-issue-nit-bundle instead of
falling through to the feature template.

Closes #137

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author
Contributor

Self-Review: LGTM

Diff: 1 file, +3/-2 lines

Findings: None. The change is a single case addition following the existing pattern. Spacing realignment on adjacent lines keeps the assignment operators visually aligned. No logic changes to existing paths. Mergeable with no conflicts.

## Self-Review: LGTM **Diff:** 1 file, +3/-2 lines **Findings:** None. The change is a single case addition following the existing pattern. Spacing realignment on adjacent lines keeps the assignment operators visually aligned. No logic changes to existing paths. Mergeable with no conflicts.
Author
Contributor

PR #139 Review

DOMAIN REVIEW

Tech stack: Bash shell script (PreToolUse hook for Claude Code).

Correctness:

  • The new Nit-Bundle|nit-bundle) case is syntactically correct and placed before the * fallback, preserving case statement semantics.
  • The target template template-issue-nit-bundle exists in pal-e-docs (id: 628, tagged active), so the hook will successfully fetch and validate against it.
  • The naming convention template-issue-{type} is followed consistently.
  • Column alignment on the ;; terminators is a reasonable readability improvement.

Header comment drift (NIT):
Lines 5-8 of the file list the recognized types:

#   Bug   -> template-issue-bug
#   Feature -> template-issue-feature (default)
#   Spike -> template-issue-spike

The new Nit-Bundle type is not reflected here. The doc block is now stale -- it will mislead anyone reading the header to understand what types this hook recognizes.

BLOCKERS

None. This is a 3-line change to a shell case statement. No new functionality requiring tests (the hook's existing logic is unchanged -- only the mapping table gains an entry). No user input validation concerns (the ISSUE_TYPE is extracted from the agent's own issue body, matched against fixed strings). No secrets. No DRY violations.

NITS

  1. Stale header comment -- Lines 5-8 should include Nit-Bundle -> template-issue-nit-bundle in the type mapping documentation. This is the kind of drift that compounds -- every new type addition that skips the header makes it less trustworthy.

  2. Inconsistent alignment on Bug line -- The Bug|bug) case (line 34) was not re-aligned with the new column width. After the change, Bug|bug) uses tighter spacing while Spike|spike) and Nit-Bundle|nit-bundle) are padded to align ;; terminators. Either align all four cases or leave the original spacing. The current state is a mix.

SOP COMPLIANCE

  • Branch named after issue (137-add-nit-bundle-type-to-check-issue-templ -- starts with 137)
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related section references plan slug -- PR body says Closes #137 but does not reference plan-pal-e-agency. Per template-pr-body: "Both are required. The Forgejo issue is the spec; the plan slug is the strategic context."
  • No secrets committed
  • No unnecessary file changes (single file, exactly scoped to the issue)
  • Commit messages are descriptive (implied by PR title)

PROCESS OBSERVATIONS

  • Change failure risk: Very low. This is an additive case branch in a fail-open hook. Even if the template slug were wrong, the hook would fail open (line 17: trap 'exit 0' ERR). The template has been verified to exist.
  • Deployment frequency: No friction. Single-file, single-line-of-logic change.
  • Traceability gap: The missing plan slug in Related is a minor process gap. Issue #137 presumably maps to a phase in plan-pal-e-agency, but the PR does not make that connection explicit.

VERDICT: APPROVED

The code change is correct, minimal, and safe. The two nits (stale header comment, inconsistent alignment) and the missing plan slug in Related are non-blocking. The header comment nit should be tracked for the next nit-bundle on this repo.

## PR #139 Review ### DOMAIN REVIEW **Tech stack:** Bash shell script (PreToolUse hook for Claude Code). **Correctness:** - The new `Nit-Bundle|nit-bundle)` case is syntactically correct and placed before the `*` fallback, preserving case statement semantics. - The target template `template-issue-nit-bundle` exists in pal-e-docs (id: 628, tagged `active`), so the hook will successfully fetch and validate against it. - The naming convention `template-issue-{type}` is followed consistently. - Column alignment on the `;;` terminators is a reasonable readability improvement. **Header comment drift (NIT):** Lines 5-8 of the file list the recognized types: ``` # Bug -> template-issue-bug # Feature -> template-issue-feature (default) # Spike -> template-issue-spike ``` The new `Nit-Bundle` type is not reflected here. The doc block is now stale -- it will mislead anyone reading the header to understand what types this hook recognizes. ### BLOCKERS None. This is a 3-line change to a shell case statement. No new functionality requiring tests (the hook's existing logic is unchanged -- only the mapping table gains an entry). No user input validation concerns (the `ISSUE_TYPE` is extracted from the agent's own issue body, matched against fixed strings). No secrets. No DRY violations. ### NITS 1. **Stale header comment** -- Lines 5-8 should include `Nit-Bundle -> template-issue-nit-bundle` in the type mapping documentation. This is the kind of drift that compounds -- every new type addition that skips the header makes it less trustworthy. 2. **Inconsistent alignment on Bug line** -- The `Bug|bug)` case (line 34) was not re-aligned with the new column width. After the change, `Bug|bug)` uses tighter spacing while `Spike|spike)` and `Nit-Bundle|nit-bundle)` are padded to align `;;` terminators. Either align all four cases or leave the original spacing. The current state is a mix. ### SOP COMPLIANCE - [x] Branch named after issue (`137-add-nit-bundle-type-to-check-issue-templ` -- starts with `137`) - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related section references plan slug -- PR body says `Closes #137` but does not reference `plan-pal-e-agency`. Per `template-pr-body`: "Both are required. The Forgejo issue is the spec; the plan slug is the strategic context." - [x] No secrets committed - [x] No unnecessary file changes (single file, exactly scoped to the issue) - [x] Commit messages are descriptive (implied by PR title) ### PROCESS OBSERVATIONS - **Change failure risk:** Very low. This is an additive case branch in a fail-open hook. Even if the template slug were wrong, the hook would fail open (line 17: `trap 'exit 0' ERR`). The template has been verified to exist. - **Deployment frequency:** No friction. Single-file, single-line-of-logic change. - **Traceability gap:** The missing plan slug in Related is a minor process gap. Issue #137 presumably maps to a phase in `plan-pal-e-agency`, but the PR does not make that connection explicit. ### VERDICT: APPROVED The code change is correct, minimal, and safe. The two nits (stale header comment, inconsistent alignment) and the missing plan slug in Related are non-blocking. The header comment nit should be tracked for the next nit-bundle on this repo.
forgejo_admin deleted branch 137-add-nit-bundle-type-to-check-issue-templ 2026-03-21 17:28:04 +00:00
Sign in to join this conversation.
No description provided.