Skip to content

ci: Skip heavy test jobs for docs-only PRs - #2391

Merged
cgwalters merged 1 commit into
bootc-dev:mainfrom
gursewak1997:ci-skip-md-docs
Aug 19, 2026
Merged

cgwalters merged 1 commit into
bootc-dev:mainfrom
gursewak1997:ci-skip-md-docs

Conversation

@gursewak1997

Copy link
Copy Markdown
Contributor

Skip heavy test jobs for docs-only PRs

@bootc-bot
bootc-bot Bot requested a review from ckyrouac August 18, 2026 23:37
Johan-Liebert1
Johan-Liebert1 previously approved these changes Aug 19, 2026

@Johan-Liebert1 Johan-Liebert1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread .github/workflows/ci.yml Outdated
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
FILES=$(gh api "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, perhaps cleaner to interact in terms of git diff? Aren't there existing actions/tools for this?

Comment thread .github/workflows/ci.yml Outdated
Assisted-by: AI
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
@cgwalters
cgwalters merged commit 316248c into bootc-dev:main Aug 19, 2026
18 checks passed
@cgwalters

Copy link
Copy Markdown
Collaborator

So I got suspicious when merging this because currently our CI was down...and this job changed CI itself, not just docs, so CI should have been failing.

Basically compute-ci-level doesn't run actions/checkout, so when docs-check calls
gh pr diff "$PR" --name-only, gh can't find a local git repo to infer
owner/repo from and fails:

failed to run git: fatal: not a git repository (or any parent up to mount point /)

That error goes to stderr, so stdout is empty. Piped into
grep -qvE '\.md$', empty input means "found no non-md line", so grep
exits 1 and the if falls into the else branch — docs_only gets set to
true unconditionally, regardless of what actually changed.

I have to emphasize just how absolutely terrible of a language shell script is, and this is a perfect example of that.

We need to add a checkout step and also correctly test exit status, which is best by separating fetch from grep.

@cgwalters

Copy link
Copy Markdown
Collaborator

Since we're redoing this let's also please try harder to clean up this code, I think we should only be testing the docs conditional once for example.

Also what may work better is to factor out this stuff into e.g. ci/analyze or so as an independent script (not embedded in the yaml) so we can more easily run it outside of GHA to test etc.

@cgwalters

Copy link
Copy Markdown
Collaborator

Bigger picture one thing we could try to do is factor out most of this logic into https://github.com/bootc-dev/actions perhaps and try to apply it consistently across all of the repos that way.

(And when we do that we should write more of it in typescript probably, not bash)

@gursewak1997

Copy link
Copy Markdown
Contributor Author

This should have been caught earlier but starting the factoring out: bootc-dev/actions#51

@cgwalters

Copy link
Copy Markdown
Collaborator

Sorry I'm giving slightly conflicting messages but it may make the most sense to do a quick PR that tries to fix this in the existing shell script and land that ASAP, but have a parallel effort to do the generic actions and js logic?

@gursewak1997

Copy link
Copy Markdown
Contributor Author

That was my take but then I went on an ambitious tangent and thought we could land something in actions soon (today) and get something up and running. Let me fix this and make sure we are not skipping CI:/

@cgwalters

Copy link
Copy Markdown
Collaborator

No worries, and thanks for working on this!

BTW one thing I've been doing because this stuff is hard to test is basically having agents force push to https://github.com/bootc-dev/ci-sandbox and testing things there.

(Though we should perhaps make separate $username-ci-sandbox repos to avoid conflict?)

gursewak1997 added a commit to gursewak1997/bootc that referenced this pull request Aug 19, 2026
The previous attempt (merged in bootc-dev#2391) failed silently because
compute-ci-level had no checkout step, so gh pr diff could not
infer the repository and produced empty output. The pipe into
grep swallowed the error, setting docs_only=true for all PRs.

Fix by adding a sparse checkout for PR events, separating the
gh pr diff fetch from the grep so failures abort via set -euo
pipefail, and guarding empty output explicitly.

Assisted-by: AI
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
gursewak1997 added a commit to gursewak1997/bootc that referenced this pull request Aug 19, 2026
The previous attempt (merged in bootc-dev#2391) failed silently because
compute-ci-level had no checkout step, so gh pr diff could not
infer the repository and produced empty output. The pipe into
grep swallowed the error, setting docs_only=true for all PRs.

Fix by adding a sparse checkout for PR events, separating the
gh pr diff fetch from the grep so failures abort via set -euo
pipefail, and guarding empty output explicitly.

Assisted-by: AI
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
gursewak1997 added a commit to gursewak1997/bootc that referenced this pull request Aug 19, 2026
The previous attempt (merged in bootc-dev#2391) failed silently because
compute-ci-level had no checkout step, so gh pr diff could not
infer the repository and produced empty output. The pipe into
grep swallowed the error, setting docs_only=true for all PRs.

Fix by adding a sparse checkout for PR events, separating the
gh pr diff fetch from the grep so failures abort via set -euo
pipefail, and guarding empty output explicitly.

Assisted-by: AI
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
cgwalters pushed a commit that referenced this pull request Aug 20, 2026
The previous attempt (merged in #2391) failed silently because
compute-ci-level had no checkout step, so gh pr diff could not
infer the repository and produced empty output. The pipe into
grep swallowed the error, setting docs_only=true for all PRs.

Fix by adding a sparse checkout for PR events, separating the
gh pr diff fetch from the grep so failures abort via set -euo
pipefail, and guarding empty output explicitly.

Assisted-by: AI
Signed-off-by: gursewak1997 <gursmangat@gmail.com>
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