feat: codebase-analysis skill
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.
This commit is contained in:
@@ -0,0 +1,97 @@
|
||||
# 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 |
|
||||
Reference in New Issue
Block a user