feat(licensing): bootstrap 4 rule blocks in go-licensing-guide#14
Merged
Conversation
Restructures the 4 MUST sections in docs/go-licensing-guide.md into canonical `### RULE` blocks. Same template as PRs #2-5, #8, #10. Rules added: - go-licensing/license-file-required (MUST, judgment) Public Go repos MUST have a root LICENSE file. Private/internal repos exempt. Detection requires file-existence check — not ast-grep. - go-licensing/readme-license-section-required (MUST, judgment) Public README must have a ## License H2 section pointing to LICENSE. Markdown structure check — not ast-grep. - go-licensing/source-file-header-required (MUST, addlicense) All non-vendor *.go files need the 3-line BSD-2 header. Enforcement delegates to 'addlicense -check' via make precommit (canonical tool; already wired into the toolchain). - go-licensing/copyright-year-discipline (MUST, judgment) No bulk year-updates for trivial changes; no future/non-numeric years. PR-scope diff inspection — partial ast-grep detection possible for the future-year case but bulk-update trigger needs diff-scope reasoning. rules/index.json: 42 -> 46 Walker drift detected by PR #13's check-index target during first precommit run — regenerated rules/index.json and re-ran precommit clean. New guard working as designed. Pre-emptive checks (lessons from #6, #8): no personal vault paths, no trading-domain terms, no internal contradictions, all cross-refs resolve. License-assistant agent exists at agents/license-assistant.md.
There was a problem hiding this comment.
PR Review Complete
Summary
Reviewed the feat(licensing): bootstrap 4 rule blocks in go-licensing-guide PR which adds 4 new RULE blocks to docs/go-licensing-guide.md and corresponding entries to rules/index.json. The rules (go-licensing/license-file-required, go-licensing/readme-license-section-required, go-licensing/source-file-header-required, go-licensing/copyright-year-discipline) are well-defined with proper enforcement mechanisms and alignment between doc anchors and index entries. The source-file-header-required rule correctly leverages the existing addlicense -check tool already wired into make precommit.
Must Fix (Critical)
None identified.
Should Fix (Important)
- Index insertion at position 188: The 4 new entries in
rules/index.jsonwere inserted mid-file at line 188 rather than appended at the end. This creates unnecessary merge conflict surface if other PRs are editing the index simultaneously. Future bootstrap PRs should append new entries at the end of the array.
Nice to Have (Optional)
None.
Plan Concerns Addressed
- Index position — Valid concern. Not blocking but noted above.
- Mid-section RULE placement — Not an issue. RULE blocks are correctly placed within their logical sections per the guide's structure.
- Judgment-only enforcement for copyright-year-discipline — Correct design. Bulk-update detection requires PR-scope reasoning beyond mechanical ast-grep patterns.
{
"verdict": "approve",
"summary": "The PR correctly bootstraps 4 licensing RULE blocks into go-licensing-guide.md with aligned index.json entries. All rule definitions are sound, enforcement mechanisms are appropriate (tooled for addlicense, judgment for others), and doc-anchor alignment is correct. One minor merge-hygiene concern: new index entries were inserted mid-file rather than appended at end.",
"comments": [
{
"file": "rules/index.json",
"line": 191,
"severity": "major",
"message": "Should Fix: New licensing rule entries inserted at position 188 rather than appended at array end. This creates unnecessary merge conflict risk with concurrent PRs editing the index. Append new entries at the end of the array instead."
}
],
"concerns_addressed": [
"correctness: rules/index.json - 4 new entries added with correct structure and alignment to doc anchors",
"correctness: docs/go-licensing-guide.md - RULE blocks placed in appropriate logical sections",
"correctness: copyright-year-discipline rule correctly uses judgment enforcement (ast-grep cannot detect bulk-update intent)"
]
}
bborbe
added a commit
that referenced
this pull request
Jun 2, 2026
Restructures the godoc style guide with canonical `### RULE` blocks for its enforceable conventions. Mirrors PRs #2-5, #8, #10, #14. Rules added (rules/index.json: 46 -> 50): - go-doc/exported-item-must-have-comment (MUST, judgment) Every exported function/type/const/var needs a doc comment. Mechanical follow-up: revive's 'exported' rule. - go-doc/comment-starts-with-name (MUST, judgment) Doc comment first word must match the identifier name. Mechanical follow-up: revive (or ast-grep via the PR #11 recipe). - go-doc/package-comment-in-doc-go (SHOULD, judgment) Package comment belongs in doc.go to survive refactors of feature files. - go-doc/third-person-no-signature-repeat (SHOULD, judgment) No first-person prose; don't restate the signature. All examples generic (Order, Customer, Discount, Add) — no trading domain. All rule IDs unique against the 46 existing entries. godoc-assistant agent exists at agents/godoc-assistant.md. Pre-emptive checks (lessons from PRs #6, #8, #14): - No personal vault paths - No trading-domain terms - All cross-refs resolve - make build-index regenerated; check-index target passes
3 tasks
bborbe
added a commit
that referenced
this pull request
Jun 2, 2026
Restructures the 6 enforceable conventions in docs/go-architecture-patterns.md into canonical `### RULE` blocks. Mirrors PRs #2-5, #8, #10, #14, #15. Rules added (rules/index.json: 50 -> 56, new go-architecture/* family): - go-architecture/counterfeiter-directive-on-interface (MUST) Every substitutable interface needs a //counterfeiter:generate directive so go generate regenerates the fake when the interface drifts. - go-architecture/new-prefix-constructor-naming (MUST) Constructors start with New (not Create/Make/etc). Partial overlap with go-factory/no-impl-in-factory-pkg for Create*. - go-architecture/constructor-returns-interface (MUST) New* returns the interface, not *concrete struct. Hides implementation, keeps dependency direction one-way. - go-architecture/private-struct-matches-interface (SHOULD) UserService -> userService (first letter lowered). godoc pairs them; IDE outlines pair them; refactors find them with one rename. - go-architecture/no-globals-or-singletons (MUST) Service deps via constructor injection, never package-level vars. Globals break parallel tests, hide the dep graph, make refactors fragile. - go-architecture/business-logic-not-in-main (MUST) main.go is wiring; domain operations live in pkg/. Mixed main.go is untestable and unreachable from other binaries. Cross-references to existing rules (already in index, not duplicated): - go-errors/no-context-background-in-business-logic — covers context discipline - go-context/cancel-check-in-loop — covers infinite-loop ctx.Done() checks - go-time/* family — covers libtime injection All examples generic (User, UserService, Worker, log/db deps). No trading-domain terms (KafkaBrokers grep hit is server infrastructure, not trading-broker terminology — pre-existing in main.go example). No personal vault paths. Pre-emptive checks (lessons from PRs #6, #8, #14, #15): clean grep, unique rule IDs (6 new vs 50 existing), go-architecture-assistant agent exists at agents/go-architecture-assistant.md, make build-index regenerated, check-index passes.
4 tasks
bborbe
added a commit
that referenced
this pull request
Jun 2, 2026
…nal-options) Second multi-doc bootstrap PR — same shape as PR #17. 3 docs (~500-770 lines), 6 rules, single bot review cycle. Rules added (rules/index.json: 62 -> 68): go-service-impl/* (owner: go-architecture-assistant) - no-context-object-injection (MUST) — never bundle deps in a *Context / *Deps struct passed through methods. Constructor injection makes the dep set visible at the type signature and minimal in scope. - provider-vs-registry-choice (SHOULD) — static switch for fixed compile-time sets (< ~10 types); map-based registry only for runtime extensible / plugin systems. go-k8s-crd/* (owner: go-architecture-assistant) - use-bborbe-k8s (SHOULD) — collapse ~300 lines of hand-written event-handler + adapter + store boilerplate into k8s.NewEventHandler[T] + k8s.NewResourceEventHandler[T] from github.com/bborbe/k8s. - generated-client-not-dynamic (MUST) — first-party CRDs use the typed clientset from hack/update-codegen.sh, not client-go/dynamic. Dynamic client is for unknown schemas (admin tools, generic operators); known-schema use throws away every type-safety guarantee. go-functional-options/* (owner: go-quality-assistant) - singular-option-type (SHOULD) — XxxOption (singular fn type) + XxxOptions (plural struct). Industry-standard pair-naming. - with-prefix-option-functions (SHOULD) — option constructors prefixed With*. Uniform prefix makes the option-list call site self-describing. CLAUDE.md doc-agent table updated with all 3 new mappings. Pre-emptive checks (lessons from PRs #6, #8, #14, #17): - No personal vault paths - One trading-domain leak introduced in my service-impl example (ProcessMarket/Limit/Stop — order-type terminology) caught by pre-push grep + genericised to ProcessImage/Video/Document. Pre-existing OrderType/MarketOrder/LimitOrder/StopOrder references in the original doc are NOT changed in this PR (out of scope; the bot may flag them as pre-existing). - All 6 rule IDs unique against 62 existing entries - All 3 owner agents exist - make build-index regenerated; check-index passes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bootstraps `docs/go-licensing-guide.md` — 4 MUST sections restructured into canonical `### RULE` blocks. Mirrors PRs #2-5, #8, #10.
`rules/index.json`: 42 → 46 entries.
Rules added
Notes
PR #13's check-index caught the walker drift
First `make precommit` failed with the new `check-index` target catching the unregen'd `rules/index.json`. Ran `make build-index`, re-ran precommit clean. Drift guard working as designed — the gap that turned PR #9 into a two-PR walker-regen catch-up is now closed at the precommit boundary.
Pre-emptive checks
Test plan