Rodin
38a9d66072
8-phase methodology for extracting architectural conventions from open source repositories: 1. Clone & Shape — measure repo dimensions 2. Import Hierarchy — find foundational packages 3. Interface Contracts — key abstractions 4. Error Handling & Quality Markers — TODOs, style 5. Unique Patterns — project-specific infrastructure 6. Git Archaeology — trace WHY decisions were made 7. PR Discussions — read the actual debates 8. Synthesis — produce analysis.md + conventions.md Includes pattern-breaks reference (4 categories of why conventions are violated, with real examples from CockroachDB, Prometheus, Temporal, Ecto, Oban). Output: Gitea repos named <project>-conventions.
2.9 KiB
Pattern Breaks: Why Conventions Are Violated
Based on git archaeology across CockroachDB, Prometheus, Temporal, Ecto, and Oban. Each category with real examples.
Category 1: Ship Behavior, Fix Plumbing Later
Example: CockroachDB bare goroutine in kvadmission
// TODO(irfansharif): Use a stopper here instead.
go func() {
ticker := time.NewTicker(10 * time.Minute)
for { select { case <-ticker.C: ... } }
}()
History: Irfan Sharif, Aug 2022. Part of the elastic CPU limiter (20+ file change). Complex new admission control system. The author tagged it with TODO in the SAME commit. Ship the behavior, fix the plumbing later.
Why it survives: Nobody touched the function for any other reason. The goroutine leaks nowhere (process-lifetime). The TODO is correct but not urgent.
For review: Worth noting, not worth blocking the PR.
Category 2: Better Tooling Exposed the Limitation
Example: CockroachDB Handle replacing RunAsyncTask
History: Feb 2025, Tobias Grieger. RunAsyncTask was correct but made every goroutine look identical in execution traces. The Handle pattern was motivated by profiling/observability, not correctness.
Key quote from PR: "It becomes an ordeal to search for a specific goroutine, since all goroutines started by the Stopper get assigned the same searchable identity."
For review: Track it. This is how progress happens.
Category 3: Removal Cost > Carrying Cost
Example: CockroachDB stale "Remove in 22.2" TODO
// TODO(ajwerner): Remove in 22.2.
SystemConfigProvider config.SystemConfigProvider
History: Feb 2022. Bridge for mixed-version clusters during gossip→span-config migration. A struct field that costs one pointer of memory. Removing it means touching StoreConfig initialization across dozens of call sites.
Why it survives: Cost of removal >> cost of carrying. The interest rate on this debt is zero.
For review: Leave it alone.
Category 4: Context Needs Different Pattern
Example: Prometheus global vars for scrape timestamp tolerance
var ScrapeTimestampTolerance = 2 * time.Millisecond
History: Started as "experimental, hidden flag" (Oct 2020). Made public after production showed 10ms tolerance saves real disk space.
Why a global: Set once at startup from a command-line flag. Read thousands of times per second in the scrape loop. Threading a config struct would add parameters to every hot-path function for a value that never changes after init.
For review: Approve it. This IS the right pattern for this context.
Summary Table
| Category | Response in Review | Example |
|---|---|---|
| Time pressure | Note, don't block | kvadmission goroutine |
| Better tooling | Track migration | Handle API |
| Zero-interest debt | Leave alone | Stale version TODOs |
| Different context | Approve | Hot-path globals |