Skip to content

ci: skip expensive jobs for docs-only changes#369

Merged
benvinegar merged 1 commit into
mainfrom
ci/skip-docs-only-expensive-jobs
May 25, 2026
Merged

ci: skip expensive jobs for docs-only changes#369
benvinegar merged 1 commit into
mainfrom
ci/skip-docs-only-expensive-jobs

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • add a shared CI change detector for docs/assets/LICENSE-only updates
  • keep PR/main/Nix workflows running a cheap detector while skipping expensive jobs when there are no code changes
  • gate Windows compatibility, typecheck/tests, package, prebuilt, binary, and Nix jobs on code changes

Testing

  • bun run format:check
  • manual detect-code-changes.sh checks in a temporary git repo for docs-only and code-change scenarios

This PR description was generated by Pi using OpenAI GPT-5

@benvinegar benvinegar force-pushed the ci/skip-docs-only-expensive-jobs branch from 8f93487 to 83a488d Compare May 25, 2026 01:18
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 25, 2026

Greptile Summary

This PR replaces per-workflow paths-ignore filters with a shared detect-code-changes.sh script and a lightweight changes gating job that runs first in each workflow, skipping all expensive jobs when only docs/assets/LICENSE files are touched.

  • A new detect-code-changes.sh script compares base and head SHAs via git diff --name-only, handles shallow clones by fetching missing commits, and writes code_changed=true/false to GITHUB_OUTPUT.
  • Three workflows (ci.yml, pr-ci.yml, nix.yml) each gain a changes job; all expensive jobs (typecheck, tests, packaging, Nix build, Windows compat) are gated with if: needs.changes.outputs.code == 'true'.
  • Each workflow correctly resolves the base SHA for its event type: github.event.before for push-to-main events and github.event.pull_request.base.sha for pull-request events.

Confidence Score: 4/5

Safe to merge — the change only affects when expensive CI jobs run, not how the project is built or tested.

The logic is sound: the script correctly handles shallow clones, zero SHAs, and both push/PR event contexts. The only issue is a pair of unreachable docs/** and assets/** alternatives in the case statement that can never be reached after the preceding docs/*/assets/* arms — cosmetic but harmless.

.github/scripts/detect-code-changes.sh has the redundant case patterns; all three workflow files look correct.

Important Files Changed

Filename Overview
.github/scripts/detect-code-changes.sh New script that diffs base/head SHAs and outputs code_changed; handles shallow clones and zero SHAs correctly, but has redundant docs/**/assets/** case patterns (unreachable after docs/*/assets/*).
.github/workflows/ci.yml Replaces paths-ignore with a changes gating job; all expensive jobs correctly depend on needs.changes.outputs.code == 'true'. BASE_SHA from github.event.before is appropriate for push events and the zero-SHA edge case is handled by the script.
.github/workflows/nix.yml Adds the same changes job pattern; correctly uses a ternary expression to pick pull_request.base.sha vs github.event.before depending on the trigger event.
.github/workflows/pr-ci.yml Adds changes job using github.event.pull_request.base.sha as the base, which is the correct anchor for a PR diff; all expensive jobs properly gated.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Push to main / PR opened] --> B[changes job\ndetect-code-changes.sh]
    B --> C{code_changed?}
    C -- false\ndocs/assets/LICENSE only --> D[All expensive jobs SKIPPED]
    C -- true\ncode files touched --> E[validate / pr-validate]
    C -- true --> F[tty-smoke]
    C -- true --> G[pack-npm / prebuilt-npm]
    C -- true --> H[build-bin]
    C -- true --> I[windows-compat]
    C -- true --> J[Nix package]
    subgraph detect-code-changes.sh
        K[resolve zero base SHA → empty tree] --> L[fetch missing SHAs\nif shallow clone]
        L --> M[git diff --name-only\nbase..head]
        M --> N{any non-docs path?}
        N -- yes --> O[code_changed=true]
        N -- no --> P[code_changed=false]
    end
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
.github/scripts/detect-code-changes.sh:22
In bash `case` patterns, `*` matches any character sequence **including** `/`, so `docs/*` already matches `docs/subfolder/file.png` and any deeper path. The `docs/**` and `assets/**` alternatives are therefore dead branches — they can never be reached because `docs/*` always matches first and exits the case block. The same applies to `assets/**` following `assets/*`.

```suggestion
    *.md | docs/* | assets/* | LICENSE)
```

Reviews (1): Last reviewed commit: "ci: skip expensive jobs for docs-only ch..." | Re-trigger Greptile

Comment thread .github/scripts/detect-code-changes.sh Outdated
local path="$1"

case "$path" in
*.md | docs/* | docs/** | assets/* | assets/** | LICENSE)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 In bash case patterns, * matches any character sequence including /, so docs/* already matches docs/subfolder/file.png and any deeper path. The docs/** and assets/** alternatives are therefore dead branches — they can never be reached because docs/* always matches first and exits the case block. The same applies to assets/** following assets/*.

Suggested change
*.md | docs/* | docs/** | assets/* | assets/** | LICENSE)
*.md | docs/* | assets/* | LICENSE)
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/scripts/detect-code-changes.sh
Line: 22

Comment:
In bash `case` patterns, `*` matches any character sequence **including** `/`, so `docs/*` already matches `docs/subfolder/file.png` and any deeper path. The `docs/**` and `assets/**` alternatives are therefore dead branches — they can never be reached because `docs/*` always matches first and exits the case block. The same applies to `assets/**` following `assets/*`.

```suggestion
    *.md | docs/* | assets/* | LICENSE)
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — simplified the Bash case pattern to remove the unreachable docs/** / assets/** alternatives while preserving recursive matching via docs/* and assets/*.

This comment was generated by Pi using OpenAI GPT-5

@benvinegar benvinegar force-pushed the ci/skip-docs-only-expensive-jobs branch from 83a488d to 8d9f9cb Compare May 25, 2026 01:20
@benvinegar benvinegar merged commit 3c2c660 into main May 25, 2026
8 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.

1 participant