Conversation
…n + partial pre-existence acknowledged Three substantive review findings absorbed: 1. Cross-workflow `needs:` is invalid — `needs:` works only within single workflow file. Architecture corrected to specify three options (Architecture A: reusable workflow via workflow_call; Architecture B: per-workflow detect-job; Architecture C: single consolidated workflow). Architecture A recommended (matches sibling-repo pattern + preserves separation). 2. .yaml → .yml extension throughout — repo's existing workflows consistently use .yml; one-off .yaml suffix would create drift. 3. Problem-statement acknowledged partial pre-existence — gate.yml already skips on docs-only via path-gate; codeql.yml short-circuits pure docs; backlog-only changes have backlog-index-integrity. Row generalizes the partial pattern into uniform mechanism, NOT from-zero gating. The B-0142-stale-cross-references in some threads were merge-ordering false positives (B-0142 not present at PR-eval time; merged later as #1349) — handled in resolve-thread comments.
AceHack
deleted the
backlog/B-0157-followup-yml-extension-and-stale-problem-statement
branch
There was a problem hiding this comment.
Pull request overview
Updates backlog row B-0157 to correct post-merge review findings about GitHub Actions workflow gating semantics and align documentation with existing CI conventions.
Changes:
- Replaces invalid cross-workflow
needs:framing with a 3-architecture explanation (reusable workflow / per-workflow detect-job / consolidated workflow). - Normalizes workflow filename references from
.yamlto.ymland adds a “partial pre-existence” note acknowledging existing gating. - Clarifies which prior findings were merge-ordering false positives vs. substantive corrections.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Substantive post-merge review findings on B-0157 absorbed:
Cross-workflow
needs:is invalid (line 67 P2) — GitHub Actions `needs:` works only within single workflow file (`jobs.<job_id>.needs`), NOT across workflow files. Architecture correction: row now specifies three options (Architecture A: reusable workflow via `workflow_call` — recommended; Architecture B: per-workflow detect-job; Architecture C: single consolidated workflow).`.yaml` → `.yml` extension (lines 41 + 66) — all existing workflows use `.yml`; one-off `.yaml` would create drift. Updated throughout.
Stale problem statement (line 35) — `gate.yml` already skips on docs-only via `path-gate`; `codeql.yml` short-circuits pure docs; `backlog-index-integrity` exists. Row now acknowledges partial pre-existence — generalizes pattern, doesn't introduce from zero.
The remaining 3 threads (lines 8, 25, 62) were merge-ordering false positives — B-0142 wasn't present at PR-eval time but merged later as #1349. Resolved with explanation.
Test plan
.yamlreferences →.yml