chore: ignore non-code files from triggering builds#3175
Closed
rahul31124 wants to merge 1 commit into
Closed
Conversation
Contributor
Reviewer's GuideIntroduces a Detect Changes gatekeeper job in the pull-request GitHub Actions workflow that always runs, classifies PRs as code vs non-code based on a git diff, and conditionally runs or skips the expensive platform build/screenshot jobs while still reporting required checks for all PRs. Flow diagram for Detect Changes gatekeeper logicflowchart TD
start([Start Detect Changes job])
diff[Run git diff --name-only base_sha..head_sha]
filterExt[Filter out files by extension md, png, jpg, jpeg, gif, svg, webp, txt, arb]
filterDocs[Filter out files under docs directory]
anyFiles{Any files remaining?}
isCodeTrue[Set output is_code=true]
isCodeFalse[Set output is_code=false]
endTrue([Downstream jobs see is_code=true])
endFalse([Downstream jobs see is_code=false])
start --> diff --> filterExt --> filterDocs --> anyFiles
anyFiles -->|Yes| isCodeTrue --> endTrue
anyFiles -->|No| isCodeFalse --> endFalse
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
grep -v '^docs/'filter only ignores files in a top-leveldocs/directory, whereas the previouspaths-ignore: docs/**would also ignore nested docs paths (e.g.packages/foo/docs/...); if such patterns exist in this repo, consider broadening the path filter to preserve the previous behavior. - Many jobs repeat the same
if: needs.changes.outputs.is_code == 'true'on each step and add a nearly identicalNon-Code Changestep; you could simplify this by using a job-levelif:for code-only jobs or extracting a reusable step/action to avoid duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `grep -v '^docs/'` filter only ignores files in a top-level `docs/` directory, whereas the previous `paths-ignore: docs/**` would also ignore nested docs paths (e.g. `packages/foo/docs/...`); if such patterns exist in this repo, consider broadening the path filter to preserve the previous behavior.
- Many jobs repeat the same `if: needs.changes.outputs.is_code == 'true'` on each step and add a nearly identical `Non-Code Change` step; you could simplify this by using a job-level `if:` for code-only jobs or extracting a reusable step/action to avoid duplication.
## Individual Comments
### Comment 1
<location path=".github/workflows/pull-request.yml" line_range="24-33" />
<code_context>
+ outputs:
+ is_code: ${{ steps.check.outputs.is_code }}
+ steps:
+ - uses: actions/checkout@v6
+ with:
+ fetch-depth: 0
+
+ - id: check
+ run: |
+ # Diff the actual commit SHAs.
+ # grep -vE removes your media/doc extensions. grep -v removes the docs folder.
+ # If anything is left over, it's code. If the output is empty, it's just docs.
+ if git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} | grep -vE '\.(md|png|jpg|jpeg|gif|svg|webp|txt|arb)$' | grep -v '^docs/'; then
+ echo "is_code=true" >> $GITHUB_OUTPUT
+ else
+ echo "is_code=false" >> $GITHUB_OUTPUT
+ fi
+
common:
name: Common Build
+ needs: changes
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
+ if: needs.changes.outputs.is_code == 'true'
- name: Common Workflow
</code_context>
<issue_to_address>
**suggestion (performance):** Consider moving the `if` condition from steps to the job level where possible.
Since every significant step in jobs like `common`, `android`, and `ios` is already guarded with `if: needs.changes.outputs.is_code == 'true'`, you could instead move this condition to the job level and introduce a small separate job for the non-code path. This would avoid spinning up runners for jobs that immediately skip all steps and make the code/non-code behavior clearer at a glance. Optional, but would simplify the workflow and reduce unnecessary resource usage.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+24
to
+33
| - uses: actions/checkout@v6 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - id: check | ||
| run: | | ||
| # Diff the actual commit SHAs. | ||
| # grep -vE removes your media/doc extensions. grep -v removes the docs folder. | ||
| # If anything is left over, it's code. If the output is empty, it's just docs. | ||
| if git diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} | grep -vE '\.(md|png|jpg|jpeg|gif|svg|webp|txt|arb)$' | grep -v '^docs/'; then |
Contributor
There was a problem hiding this comment.
suggestion (performance): Consider moving the if condition from steps to the job level where possible.
Since every significant step in jobs like common, android, and ios is already guarded with if: needs.changes.outputs.is_code == 'true', you could instead move this condition to the job level and introduce a small separate job for the non-code path. This would avoid spinning up runners for jobs that immediately skip all steps and make the code/non-code behavior clearer at a glance. Optional, but would simplify the workflow and reduce unnecessary resource usage.
Contributor
Build StatusBuild successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/24307223668/artifacts/6392888206. ScreenshotsAndroid Screenshots
iPhone Screenshots
iPad Screenshots
|
||||||||||||||||||||||||
-1_instruments_screen.png?raw=true)
-2_nav_drawer.png?raw=true)
-3_accelerometer.png?raw=true)
-4_power_source.png?raw=true)
-5_multimeter.png?raw=true)
-6_wave_generator.png?raw=true)
-7_oscilloscope.png?raw=true)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3168
Changes
Previously,
paths-ignorewas used to skip CI builds for documentation and media files.Since Android and iOS builds are marked as required checks, skipping these jobs meant no status was reported, which blocked the Merge button even for documentation-only PRs.
Solution
Added a Detect Changes gatekeeper job that checks the PR diff.
Instead of skipping the workflow, heavy build/test steps are skipped for non-code changes while still reporting successful CI checks.
How It Works
Detect Changes Logic
The Detect Changes job determines whether a PR contains code changes or only documentation/media files.
Git Diff Check
git diff --name-onlybetween the PR base SHA and head SHA.Filtering Non-Code Files
The file list is filtered using
grep:.md,.png,.jpg,.svg,.txt,.arbdocs/directoryDecision
is_code = true(code changes detected)is_code = false(documentation/media only)CI Workflow Behavior
Doc-only changes
Code changes
Mixed changes
Result
I tested the changes in my fork: rahul31124#35
The CI workflow runs successfully and required checks are reported correctly.
Documentation-only changes now skip heavy build steps while still completing the required CI jobs.
As a result, PR #3150 can now be merged.
Screenshots / Recordings
N/A
Checklist:
constants.dartor localization files instead of hard-coded values.dart formator the IDE formatter.flutter analyzeand tests run influtter test.Summary by Sourcery
Introduce a change-detection gate in the pull request workflow so required CI jobs always report status while heavy build steps are skipped for non-code changes.
Build:
CI: