Skip to content

Skip full lint for observability-only changes - #4661

Merged
lukemelia merged 1 commit into
mainfrom
cs-10958-ci-lint-path-filter
May 5, 2026
Merged

Skip full lint for observability-only changes#4661
lukemelia merged 1 commit into
mainfrom
cs-10958-ci-lint-path-filter

Conversation

@lukemelia

@lukemelia lukemelia commented May 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds paths-ignore: ['packages/observability/**'] to ci-lint.yaml's pull_request: trigger so observability-only PRs skip the ~5-minute lint job of the other 19 packages. The now-redundant Lint Observability step is removed from ci-lint.yaml.

Coverage moves to a new dedicated lint job in observability-diff.yml that runs on every observability PR (including from forks — no AWS credentials needed) and executes the full lint script: shellcheck, jq, prettier YAML check (no longer skipped), and the manifest/secret regression checks.

Why this approach

A previous iteration of this PR introduced per-package gating via dorny/paths-filter in ci-lint.yaml. Review surfaced two correctness regressions vs. the existing pattern (workspace-dep changes wouldn't trigger dependent packages' lint; e.g. a breaking type change in runtime-common would not retrigger Host's ember-tsc --noEmit) and unnecessary unconditional Boxel UI / Boxel Icons builds. The simpler paths-ignore deny-list avoids both: every package other than observability still runs the full existing lint flow on every PR, exactly as it does on main today.

The original objection to paths-ignore was that observability-diff.yml's lint step set PRETTIER_SKIP=1, so the prettier YAML check would never run on observability-only PRs. This PR fixes that root cause by adding a separate lint job (no PRETTIER_SKIP) inside observability-diff.yml.

Deny-list rather than allow-list because the failure modes are asymmetric: forgetting to add a new package to an allow-list silently skips lint for that package; running lint unnecessarily on a deny-list miss is harmless. Observability is the only package with a self-contained dedicated workflow, so it's the only entry on the deny-list.

What lint coverage looks like after this PR

PR touches ci-lint.yaml runs? observability-diff.yml::lint runs? observability-diff.yml::diff runs?
only packages/observability/** skipped yes (incl. prettier) yes (live-staging validate; non-fork only)
only other packages yes (existing 19 lints) not triggered not triggered
both yes yes yes (non-fork only)
fork PR, observability only skipped yes (incl. prettier) skipped (fork guard)
push to main yes not triggered not triggered

The diff job's existing lint step is renamed to "Lint (with live-staging validate)" and retains GRAFANACTL_VALIDATE_ENV=staging plus PRETTIER_SKIP=1 (since the new sibling lint job already covers prettier).

Risk check

  • Branch protection on main has no required_status_checks set in either classic protection or rulesets, so a skipped lint job on observability-only PRs won't block merges.
  • New non-observability packages: adding a pnpm run lint script still requires adding a Lint <Package> step in ci-lint.yaml — same as before this PR. No new fragility.
  • Fork PRs touching observability: covered by the new lint job (no AWS access required), so coverage matches non-fork PRs minus the live-staging validate.

Test plan

  • PR touching only packages/observability/**ci-lint.yaml is skipped; observability-diff.yml's lint and diff jobs both run; prettier check passes/fails on YAML formatting issues.
  • PR touching only packages/host/**ci-lint.yaml runs every existing Lint <Package> step; observability-diff.yml is not triggered.
  • PR touching both observability and host → ci-lint.yaml runs (full sweep), observability-diff.yml::lint and ::diff both run.
  • Fork PR touching only observability → ci-lint.yaml skipped, observability-diff.yml::lint runs, ::diff skipped via fork guard.
  • Introduce a YAML formatting violation in packages/observability/** → caught by observability-diff.yml::lint (the prettier check that previously didn't run on observability-only PRs).

🤖 Generated with Claude Code

This comment was marked as outdated.

@lukemelia lukemelia changed the title CS-10958: skip ci-lint for observability-only PRs Base CI linting on changes May 5, 2026
@lukemelia
lukemelia force-pushed the cs-10958-ci-lint-path-filter branch from 3da821b to fdfb823 Compare May 5, 2026 18:07
@github-actions

github-actions Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Observability diff (vs staging)

No dashboard / folder changes detected against the staging Grafana.

(Run: https://github.com/cardstack/boxel/actions/runs/25394273650)

Adds paths-ignore: ["packages/observability/**"] to ci-lint.yaml's
pull_request trigger so observability-only PRs skip the ~5-min lint job
of the other 19 packages. Removes the now-redundant `Lint Observability`
step from ci-lint.yaml.

Coverage moves to a new dedicated `lint` job in observability-diff.yml,
which runs on every observability PR (including from forks — no AWS
credentials needed) and executes the full lint script: shellcheck, jq,
prettier YAML check, and the manifest/secret regression checks. The
existing `diff` job retains its own lint step (renamed to "Lint (with
live-staging validate)") so non-fork PRs still get GRAFANACTL_VALIDATE_ENV
running grafanactl resources validate against live staging Grafana;
PRETTIER_SKIP=1 stays there since the new lint job already covers prettier.

Why deny-list (paths-ignore) over allow-list (paths): the failure mode of
forgetting to add a new package to an allow-list is silent — lint just
doesn't run for the new package. Deny-list's failure mode is benign —
lint runs unnecessarily on a PR that doesn't need it. Observability is
the only package with a self-contained dedicated workflow, so it's the
only entry on the deny-list.

Verified: main has no required_status_checks set in either classic
protection or rulesets, so the lint job being skipped on observability-
only PRs won't block PR merges.
@lukemelia
lukemelia force-pushed the cs-10958-ci-lint-path-filter branch from c02a889 to 99c1d3d Compare May 5, 2026 18:19
@lukemelia lukemelia changed the title Base CI linting on changes Skip full lint for observability-only changes May 5, 2026
@lukemelia
lukemelia requested review from a team and habdelra May 5, 2026 23:30
@lukemelia
lukemelia merged commit 933372b into main May 5, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants