justinmclean opened a new issue, #186: URL: https://github.com/apache/airflow-steward/issues/186
## Summary The canonical review category list in `criteria.md` (and the runtime enumeration in `review-flow.md` Step 4) has no entry for license / source headers. The ASF [source headers policy](https://www.apache.org/legal/src-headers.html) requires every source file to carry the standard Apache license header — this applies to all ASF projects adopting this framework. ## Problem A new project adopting the steward framework looks at three things when configuring code review: 1. `criteria.md` — the canonical category list 2. `review-flow.md` Step 4 — the runtime enumeration the skill actually loops over 3. `projects/_template/pr-management-code-review-criteria.md` — the Section anchors table they copy and populate None of these mention license headers. A project whose CI does not enforce headers (e.g. no `apache-rat` or equivalent pre-commit hook) gets zero review coverage for this ASF policy requirement. Since this is an ASF-wide requirement it should ship as a default in the framework, not be left for each adopter to discover independently. ## Proposed fix Three files need updating: **`.claude/skills/pr-management-code-review/criteria.md`** Add "License headers" to the canonical category list (after "Code quality" is a natural fit, given it is a per-file code-level requirement). **`.claude/skills/pr-management-code-review/review-flow.md`** (Step 4) Add it to the numbered category enumeration so the skill's runtime loop includes it. **`projects/_template/pr-management-code-review-criteria.md`** (Section anchors) Add a default row: | Section | Anchor URL | |---|---| | License headers | `https://www.apache.org/legal/src-headers.html` | Adopters whose project has a dedicated section in their review docs can override this with a project-specific anchor; projects with no such section get the ASF policy page as the fallback link in findings. ## Notes - The Airflow adopter config can leave this row pointing to the ASF policy URL since Airflow enforces headers via pre-commit CI (so findings will be rare in practice) — but the framework should not rely on CI coverage being present for all adopters. - This also surfaces a minor inconsistency: `criteria.md` lists 11 categories but `review-flow.md` Step 4 only enumerates 8 of them ("Quality signals to check", "Commits and PRs", "Security model" are present in `criteria.md` but absent from the Step 4 list). That discrepancy is worth resolving in the same pass. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
