Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions docs/adr/36694-decompose-buildcustomjobs-into-helpers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# ADR-36694: Decompose `buildCustomJobs` into single-responsibility helpers

**Date**: 2026-06-03
**Status**: Draft

## Context

`buildCustomJobs` in `pkg/workflow/compiler_jobs.go` had grown into a 365-line method that combined many concerns: iterating frontmatter jobs, skipping built-in/pre-activation jobs, computing dependency sets, applying implicit activation-dependency rules, extracting roughly a dozen job properties (runs-on, if, permissions, strategy, timeout, concurrency, env, container, services, continue-on-error, environment, outputs), and configuring two distinct execution modes (reusable-workflow vs. step-based). The method was a cyclomatic-complexity hotspot, and the per-property YAML handling repeated the same marshal/trim/indent boilerplate seven times. This made the orchestration flow hard to follow and individual property rules hard to test or modify in isolation, while the behavior itself was correct and needed to be preserved exactly.

## Decision

We decided to decompose `buildCustomJobs` into a thin orchestrator that delegates to focused, single-responsibility helpers, while preserving existing output and behavior byte-for-byte. The orchestrator now only iterates jobs, skips those that should not produce a custom job, builds each job, and adds it to the job manager. Detailed logic moves into named helpers grouped by concern — dependency-set precomputation (`getCustomJobDependencySets`), skip rules (`shouldSkipCustomJob`), per-job assembly (`buildCustomJob`), needs/activation handling, property extraction (one helper per property), and execution-mode configuration (`configureCustomReusableWorkflow` vs. `configureCustomJobSteps`). A shared `formatIndentedYAMLField` helper replaces the repeated marshal/indent blocks, parameterized by field name and whether to trim the trailing newline.

## Alternatives Considered

### Alternative 1: Leave the method as-is (documentation only)
We could have left the monolithic method in place and only added comments or section markers. This avoids churn and review risk, but does nothing to reduce the cyclomatic complexity, keeps the duplicated YAML boilerplate, and leaves the property rules untestable in isolation. Rejected because it does not address the underlying maintainability problem.

### Alternative 2: Introduce a dedicated `CustomJobBuilder` type
We could have extracted the logic into a separate builder struct that holds the job, config map, and compiler context, exposing methods like `withProperties()` and `withExecution()`. This would centralize state but introduces a new type and lifecycle, diverges from the surrounding free-function/method style used elsewhere in `compiler_jobs.go`, and is a larger structural change than the problem warrants. Rejected in favor of plain helper functions/methods that match existing conventions.

## Consequences

### Positive
- Cyclomatic complexity of the orchestrator drops sharply (target under ~150 lines), making the high-level flow readable at a glance.
- Each property extractor and the two execution-mode configurators can now be reasoned about and tested independently.
- The shared `formatIndentedYAMLField` helper removes ~7 copies of marshal/trim/indent boilerplate, reducing the risk of inconsistent indentation bugs.

### Negative
- The number of functions in `compiler_jobs.go` increases substantially, so navigation now requires jumping between many small functions rather than scrolling one method.
- Behavior-preserving refactors of this size carry regression risk in subtle output formatting (e.g., the `trimTrailingNewline` flag must match each field's previous behavior exactly); this relies on existing tests to catch divergence.

### Neutral
- Generated workflow YAML output shape is unchanged; this is purely an internal restructuring with no user-visible or schema impact.
- The activation-dependency exception rules (prompt-referenced and `on.needs` jobs running before activation) are preserved verbatim, now isolated in `applyAutomaticActivationDependency`.

---

*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26909802176) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
Loading
Loading