Comfy-Org / Comfy-Org/ComfyUI_frontend

Decide fallow gating: run fallow audit in CI / pre-push, and whether it replaces knip

Open
#14,769 1 comment 1 reaction 1 assignee Claimed by @DrJKL View on GitHub
area:CI/CD enhancement
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

Follow-up split out of #13284 (`docs/adr/0011-adopt-fallow.md`), requested in https://github.com/Comfy-Org/ComfyUI_frontend/pull/13284#discussion_r3717508971.

## Problem

After #13284, fallow is configured (`.fallowrc.jsonc` + `.fallow-baselines/`) and runnable (`pnpm fallow`, `pnpm fallow:audit`), but nothing enforces it. The gates still belong entirely to knip:

- CI: `pnpm knip` runs in `.github/workflows/ci-lint-format.yaml` via `.github/actions/lint-format-verify` — a required check.
- Local: `.husky/pre-push` runs `pnpm knip`, with an explicit `CI` bail-out because bot workflows populate `ComfyUI/` and contaminate knip's project glob.

So we carry two overlapping tools, and the fallow-only coverage (duplication, complexity, diff-aware `audit`) can regress unnoticed. ADR 0011 records this as undecided:

> For now Fallow runs through the `pnpm` scripts and sits alongside knip. We have not wired `fallow audit` into CI or the pre-push hook, and we have not retired knip.

## Decisions needed

1. **Where `fallow audit` runs.** Own job vs. a step inside `CI: Lint Format`. Note `ci-lint-format.yaml` is mirrored by `ci-lint-format-queue.yaml` for `merge_group`, and the workflow/job names must match on both sides for branch protection to resolve one required check — a new job needs the same treatment.
2. **Whether `audit` needs the base branch.** `fallow audit --gate new-only` diffs against the base, so CI checkout depth / base-ref fetching has to be set up accordingly.
3. **Pre-push or not.** Does `fallow audit` join `pnpm knip` in `.husky/pre-push`, replace it, or stay opt-in? Runtime on a full local tree is the deciding factor; the same `CI` bail-out applies.
4. **Knip's fate.** Retire it once fallow covers the same findings (unused files/exports/deps), or keep both permanently and document which one owns what. If it is retired, `knip.config.ts`, the `knip`/`knip:no-cache` scripts, the dependency, the pre-push hook, and `.github/actions/lint-format-verify` all need updating, and the knip exception ledger must be fully represented in `.fallowrc.jsonc`.
5. **Baseline hygiene.** Who re-saves `.fallow-baselines/*` as code is cleaned up, and how a stale baseline is prevented from masking new instances of an already-baselined pattern in an already-baselined file (a known hole called out in the ADR).

## Acceptance criteria

- A decision is recorded in `docs/adr/0011-adopt-fallow.md` (status moves off "Proposed", or a follow-up ADR supersedes it).
- Whatever is decided is implemented: the CI/pre-push wiring exists (with the `merge_group` mirror kept in sync), or the ADR states explicitly that fallow stays advisory and why.
- No duplicate enforcement is left undocumented: for each finding class (unused files, unused exports, unused deps, duplication, complexity), exactly one tool is named as the gate.

## Context

- ADR: `docs/adr/0011-adopt-fallow.md` (added in #13284), "Notes" section.
- Prior art on gradual rule enablement: #4888 (knip), #5443 (pre-push knip staging behaviour).
- Blocked on #13284 landing.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.