Fix membrowse job to run on all pushes for commit chain tracking#3488
Conversation
Co-authored-by: hathach <249515+hathach@users.noreply.github.com>
|
@membrowse does it look better ? |
Yes, it should work |
perfect, thank you |
There was a problem hiding this comment.
Pull request overview
Adjusts the Build workflow so the membrowse reporting job runs on all pushes (including doc-only pushes) to preserve MemBrowse commit-chain tracking, while keeping PR behavior gated by actual code changes.
Changes:
- Make
membrowseresilient to skipped upstream jobs by usingalways() && !cancelled()and addingcheck-pathstoneeds. - Add event-based gating: always run on
push/release/workflow_dispatch, run onpull_requestonly whencode_changed == 'true'. - Pass a computed
code_changedvalue to the reusablemembrowse-report.ymlworkflow instead of hardcodingtrue.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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') | ||
| ) |
There was a problem hiding this comment.
Using always() at the job level means membrowse will also run when cmake fails, not just when it’s skipped. In that case membrowse-report.yml will likely fall back to the identical: true path (no ELF), which can misrepresent a real code change/build failure as a doc-only/identical report. Consider additionally gating on needs.cmake.result != 'failure' (or needs.cmake.result == 'success' || needs.cmake.result == 'skipped') while keeping always() so it still runs when cmake is skipped.
| uses: ./.github/workflows/membrowse-report.yml | ||
| with: | ||
| code_changed: true | ||
| code_changed: ${{ needs.check-paths.outputs.code_changed == 'true' || github.event_name == 'release' || github.event_name == 'workflow_dispatch' }} |
There was a problem hiding this comment.
code_changed is defined as “Whether code paths changed (true) or doc-only (false)” in membrowse-report.yml, but here it’s forced to true for release/workflow_dispatch regardless of what check-paths computed. This can cause the reusable workflow to attempt artifact download/restore steps even when set-matrix/cmake were skipped (no artifacts), and it blurs the meaning of the input. Consider passing the actual needs.check-paths.outputs.code_changed == 'true' value and, if you need different behavior for release/manual runs, introduce a separate explicit input/flag for that behavior.
| code_changed: ${{ needs.check-paths.outputs.code_changed == 'true' || github.event_name == 'release' || github.event_name == 'workflow_dispatch' }} | |
| code_changed: ${{ needs.check-paths.outputs.code_changed == 'true' }} |
The
membrowsejob was being skipped on doc-only pushes because itscmakedependency was gated oncode_changed. This broke MemBrowse commit chain tracking.Changes:
always() && !cancelled()condition to runmembrowseeven whencmakeis skippedcheck-pathstoneedsarray to accesscode_changedoutputcode_changedvalue instead of hardcodedtrue💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.