policy: enforce abstraction level consistency in orchestration functions (#362)
- Add orchestration function guidance to 'Keep Abstraction Levels Consistent' section in coding policy (ja/en) — no #### nesting, integrated as paragraph - Criterion: whether branch belongs at the function's abstraction level - Concrete bad/good examples using pipeline pattern - Add 1-line behavioral guideline to architecture-reviewer persona (ja/en) - ja: 関数の責務より低い粒度の分岐が混入していたら見逃さない - en: Do not overlook branches below a function's responsibility level
This commit is contained in:
parent
f557db0908
commit
4a4a8efaf7
@ -17,6 +17,7 @@ Code is read far more often than it is written. Poorly structured code destroys
|
||||
- No "conditional approval". If there are issues, reject
|
||||
- If you find in-scope fixable issues, flag them without exception
|
||||
- Existing issues (unrelated to current change) are non-blocking, but issues introduced or fixable in this change must be flagged
|
||||
- Do not overlook branches that operate below a function's responsibility level
|
||||
|
||||
## Areas of Expertise
|
||||
|
||||
|
||||
@ -115,6 +115,36 @@ function processOrder(order) {
|
||||
}
|
||||
```
|
||||
|
||||
In orchestration functions (Step 1 → Step 2 → Step 3), pay special attention. If an individual step's internals expand with conditional branches, extract that step into a function. The criterion is not the number of branches, but **whether the branch belongs at the function's abstraction level**.
|
||||
|
||||
```typescript
|
||||
// ❌ Low-level branching exposed in orchestration function
|
||||
async function executePipeline(options) {
|
||||
const task = resolveTask(options); // Step 1: high level ✅
|
||||
|
||||
// Step 2: low-level details exposed ❌
|
||||
let execCwd = cwd;
|
||||
if (options.createWorktree) {
|
||||
const result = await confirmAndCreateWorktree(cwd, task, true);
|
||||
execCwd = result.execCwd;
|
||||
branch = result.branch;
|
||||
} else if (!options.skipGit) {
|
||||
baseBranch = getCurrentBranch(cwd);
|
||||
branch = generateBranchName(config, options.issueNumber);
|
||||
createBranch(cwd, branch);
|
||||
}
|
||||
|
||||
await executeTask({ cwd: execCwd, ... }); // Step 3: high level ✅
|
||||
}
|
||||
|
||||
// ✅ Extract details, keep abstraction levels consistent
|
||||
async function executePipeline(options) {
|
||||
const task = resolveTask(options);
|
||||
const ctx = await resolveExecutionContext(options);
|
||||
await executeTask({ cwd: ctx.execCwd, ... });
|
||||
}
|
||||
```
|
||||
|
||||
### Follow Language and Framework Conventions
|
||||
|
||||
- Write Pythonic Python, idiomatic Kotlin, etc.
|
||||
|
||||
@ -24,3 +24,4 @@
|
||||
- 軽微な問題でも後に持ち越さない。今修正できる問題は今修正させる
|
||||
- 「条件付き承認」はしない。問題があれば差し戻す
|
||||
- 既存コードの踏襲を理由にした問題の放置は認めない
|
||||
- 関数の責務より低い粒度の分岐が混入していたら見逃さない
|
||||
|
||||
@ -115,6 +115,36 @@ function processOrder(order) {
|
||||
}
|
||||
```
|
||||
|
||||
オーケストレーション関数(Step 1 → Step 2 → Step 3 と処理を並べる関数)では特に注意する。あるStepの内部に条件分岐が膨らんでいたら、そのStepを関数に抽出する。判定基準は分岐の数ではなく、**その分岐がその関数の抽象レベルに合っているか**。
|
||||
|
||||
```typescript
|
||||
// ❌ オーケストレーション関数に詳細な分岐が露出
|
||||
async function executePipeline(options) {
|
||||
const task = resolveTask(options); // Step 1: 高レベル ✅
|
||||
|
||||
// Step 2: 低レベル詳細が露出 ❌
|
||||
let execCwd = cwd;
|
||||
if (options.createWorktree) {
|
||||
const result = await confirmAndCreateWorktree(cwd, task, true);
|
||||
execCwd = result.execCwd;
|
||||
branch = result.branch;
|
||||
} else if (!options.skipGit) {
|
||||
baseBranch = getCurrentBranch(cwd);
|
||||
branch = generateBranchName(config, options.issueNumber);
|
||||
createBranch(cwd, branch);
|
||||
}
|
||||
|
||||
await executeTask({ cwd: execCwd, ... }); // Step 3: 高レベル ✅
|
||||
}
|
||||
|
||||
// ✅ 詳細を関数に抽出し、抽象度を揃える
|
||||
async function executePipeline(options) {
|
||||
const task = resolveTask(options);
|
||||
const ctx = await resolveExecutionContext(options);
|
||||
await executeTask({ cwd: ctx.execCwd, ... });
|
||||
}
|
||||
```
|
||||
|
||||
### 言語・フレームワークの作法に従う
|
||||
|
||||
- Pythonなら Pythonic に、KotlinならKotlinらしく
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user