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.
98 lines
2.9 KiB
Markdown
98 lines
2.9 KiB
Markdown
# 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
|
|
|
|
```go
|
|
// 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
|
|
|
|
```go
|
|
// 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
|
|
|
|
```go
|
|
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 |
|