Add back check-path job to workflow#3485
Conversation
… to register commit chain with membrowse
|
@membrowse let me know if this could resolve the issue in #3483 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4632441ae7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| set-matrix: | ||
| needs: [ check-paths ] | ||
| if: needs.check-paths.outputs.code_changed == 'true' | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
The new check-paths gating causes set-matrix (and therefore cmake and membrowse) to be skipped whenever code_changed is false. For pushes that only touch non-matching paths (e.g., docs), this means the workflow runs but does not execute the build or membrowse jobs, so the commit chain registration described in the comment never occurs. This regresses the intended behavior for doc-only pushes because membrowse depends on cmake and will be skipped entirely in that scenario.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the Build GitHub Actions workflow to always trigger on push/pull_request events (removing event-level paths filters) and reintroduces path-based gating via a dedicated check-paths job, with the intent of still supporting membrowse commit-chain registration.
Changes:
- Removed
on.push.pathsandon.pull_request.pathsfilters so the workflow triggers for all pushes/PRs. - Added a
check-pathsjob usingdorny/paths-filterto detect whether “code” paths changed and exposed that as an output. - Gated selected jobs (
set-matrix,build-os,zephyr) oncode_changed == 'true'and addedcheck-pathsdependency wiring.
Comments suppressed due to low confidence (1)
.github/workflows/build.yml:302
hil-hfpnow always runs for non-forked events once the workflow triggers (including doc-only pushes), since itsif:condition doesn’t checkneeds.check-paths.outputs.code_changed. This can consume self-hosted capacity unnecessarily; gate it oncode_changed == 'true'as well.
needs: [ check-paths ]
if: |
github.repository_owner == 'hathach' &&
!(github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true)
runs-on: [ self-hosted, Linux, X64, hifiphile ]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set-matrix: | ||
| needs: [ check-paths ] | ||
| if: needs.check-paths.outputs.code_changed == 'true' |
There was a problem hiding this comment.
set-matrix is skipped when code_changed is false, which causes the entire build chain (cmake -> membrowse) to be skipped as well. That contradicts the stated goal of always running this workflow to keep the membrowse commit chain updated on doc-only pushes; consider running the membrowse job unconditionally (passing code_changed through) and decoupling it from the cmake dependency for doc-only changes.
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in sizeNo entries. No changes
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The This breaks the commit chain tracking for MemBrowse on push events (even doc-only pushes should register with MemBrowse). Suggested fix - restore the original logic with membrowse:
needs: [check-paths, cmake]
if: |
always() && !cancelled() && (
github.event_name == 'push' ||
github.event_name == 'release' ||
github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' && needs.check-paths.outputs.code_changed == 'true')
)
permissions:
contents: read
actions: read
uses: ./.github/workflows/membrowse-report.yml
with:
code_changed: ${{ needs.check-paths.outputs.code_changed == 'true' || github.event_name == 'release' || github.event_name == 'workflow_dispatch'
}}
secrets: inheritKey changes:
|
|
@copilot update using suggestion below
|
Co-authored-by: hathach <249515+hathach@users.noreply.github.com>
Fix membrowse job to run on all pushes for commit chain tracking
@hathach I don't know if you noticed, but the copilot merged it's pull request into this pull request. So to get the changes to master, this pull request needs to be merged :) |
yeah, pr to pr is intended |
The pull request reintroduces the
check-pathjob to the path filter in the workflow. This ensures the workflow always runs to register the commit chain withmembrowse.