docs: add rule for when @impl functions earn their own @doc #2

2026-05-02T17:04:16Z

aweiker commented

2026-05-02 17:04:16 +00:00

Why

Gargoyle PR #519 exposed a gap: three patterns (2, 5, 10) each made partial statements about whether @impl functions should have @doc, but none owned the complete rule. A developer adding docs to callback implementations had no clear guidance on when it's appropriate vs redundant.

What

Pattern 10 (Callback Documentation Convention) now owns the full rule — both the behaviour side (@callback docs) and the implementation side (when to override @doc false)
Clear decision test: "Would this statement be true of any implementation?" → behaviour doc. If not → implementation doc.
Three trigger cases: implementation-specific guarantees, race conditions, divergent edge-case behavior
Three examples: one unnecessary override, two justified overrides
Patterns 2 and 5 replaced their partial statements with one-line cross-references to Pattern 10

## Why Gargoyle PR #519 exposed a gap: three patterns (2, 5, 10) each made partial statements about whether `@impl` functions should have `@doc`, but none owned the complete rule. A developer adding docs to callback implementations had no clear guidance on when it's appropriate vs redundant. ## What - Pattern 10 (Callback Documentation Convention) now owns the full rule — both the behaviour side (`@callback` docs) and the implementation side (when to override `@doc false`) - Clear decision test: "Would this statement be true of *any* implementation?" → behaviour doc. If not → implementation doc. - Three trigger cases: implementation-specific guarantees, race conditions, divergent edge-case behavior - Three examples: one unnecessary override, two justified overrides - Patterns 2 and 5 replaced their partial statements with one-line cross-references to Pattern 10

aweiker added 1 commit 2026-05-02 17:04:17 +00:00

docs: add rule for when @impl functions earn their own @doc edef02ed0f

Pattern 10 (Callback Documentation Convention) now owns the full rule
for callback documentation — both the behaviour side (@callback docs)
and the implementation side (when to override @doc false on @impl
functions). Patterns 2 and 5 cross-reference Pattern 10 instead of
making their own partial statements.

The test: "Would this statement be true of any implementation?" If yes,
it belongs on the @callback. If no, the implementation earns its own
@doc.

aweiker reviewed 2026-05-02 17:09:39 +00:00

aweiker left a comment

PR Review Summary

Recommendation: Approve with one minor fix

This is a well-crafted documentation PR that successfully consolidates scattered @impl / @doc guidance into a single authoritative subsection under Pattern 10. The decision test is clear, the examples are well-chosen, and the technical claims are accurate.

Status

#	Severity	Finding	Location
1	[MINOR]	Inconsistent cross-reference link text	`documentation.md:218`
2	[NIT]	TOC doesn't surface new subsection	`documentation.md:16`

Strengths

Decision test is excellent. "Would this statement be true of any implementation?" gives developers a concrete, repeatable binary heuristic
Well-chosen examples. Unnecessary override → justified guarantee → justified race condition covers the spectrum
Technical accuracy. The @impl true sets @doc false claim is correct per Elixir's Module documentation
Clean consolidation. Patterns 2 and 5 now properly defer to Pattern 10

Action Items

Standardize the link text at line 218 to match lines 459/485 (minor)
Optionally add a TOC sub-bullet for the new subsection (nit, non-blocking)

## PR Review Summary **Recommendation: Approve with one minor fix** This is a well-crafted documentation PR that successfully consolidates scattered `@impl` / `@doc` guidance into a single authoritative subsection under Pattern 10. The decision test is clear, the examples are well-chosen, and the technical claims are accurate. ### Status | # | Severity | Finding | Location | |---|----------|---------|----------| | 1 | [MINOR] | Inconsistent cross-reference link text | `documentation.md:218` | | 2 | [NIT] | TOC doesn't surface new subsection | `documentation.md:16` | ### Strengths - **Decision test is excellent.** "Would this statement be true of *any* implementation?" gives developers a concrete, repeatable binary heuristic - **Well-chosen examples.** Unnecessary override → justified guarantee → justified race condition covers the spectrum - **Technical accuracy.** The `@impl true` sets `@doc false` claim is correct per Elixir's Module documentation - **Clean consolidation.** Patterns 2 and 5 now properly defer to Pattern 10 ### Action Items 1. Standardize the link text at line 218 to match lines 459/485 (minor) 2. Optionally add a TOC sub-bullet for the new subsection (nit, non-blocking)

patterns/documentation.md

						
				@@ -1,4 +1,4 @@

				# Documentation Patterns

				Patterns extracted from the Elixir standard library source code.

aweiker commented

2026-05-02 17:09:39 +00:00

[MINOR] Inconsistent cross-reference link text.

Lines 459 and 485 use the descriptive form [Pattern 10 — implementation-side docs](#when-to-override-doc-false-on-impl-functions) but this line uses the bare [Pattern 10](...).

For consistency and discoverability, suggest:

- You're implementing a behaviour callback — `@impl true` sets `@doc false` automatically. Override only when the implementation has semantics the behaviour can't speak to (see [Pattern 10 — implementation-side docs](#when-to-override-doc-false-on-impl-functions))

[MINOR] Inconsistent cross-reference link text. Lines 459 and 485 use the descriptive form `[Pattern 10 — implementation-side docs](#when-to-override-doc-false-on-impl-functions)` but this line uses the bare `[Pattern 10](...)`. For consistency and discoverability, suggest: ```markdown - You're implementing a behaviour callback — `@impl true` sets `@doc false` automatically. Override only when the implementation has semantics the behaviour can't speak to (see [Pattern 10 — implementation-side docs](#when-to-override-doc-false-on-impl-functions)) ```

aweiker added 1 commit 2026-05-02 17:14:02 +00:00

fix: standardize cross-reference link text for Pattern 10 subsection d3adeaac25

rodin approved these changes 2026-05-02 17:18:15 +00:00

rodin left a comment

Clean consolidation. Pattern 10 now owns the full rule, cross-refs are minimal, decision test is concrete and actionable. Examples are well-chosen.

APPROVE

Clean consolidation. Pattern 10 now owns the full rule, cross-refs are minimal, decision test is concrete and actionable. Examples are well-chosen. **APPROVE**

rodin merged commit e989536bfb into master

2026-05-02 17:18:16 +00:00

rodin deleted branch docs/impl-doc-override-rule

2026-05-02 17:18:16 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rodin/elixir-patterns#2