Skip to content

Conversation

@KrystalDelusion
Copy link
Member

What are the reasons/motivation for this change?

  1. Currently (most) workflows trigger on both push and pull_request which often results in duplicated jobs when maintainers push commits to this repo that are also a part of a PR. Concurrent skipping was enabled to reduce this duplicated work, but this has problems of its own and has resulted in a few PRs being merged to main and then failing.
  2. Building and running the sanitizer checks (especially the address sanitizer) takes a long time, slowing down the whole process and potentially holding up job slots.
  3. Jobs running on main are cancelled by further commits (such as multiple consecutive PR merges). This can make it more difficult to diagnose when a failure was introduced.

Explain how this is achieved.

  1. Change (most) workflows to only trigger on pushes for the main branch. If maintainers want to run tests without making a PR they can still accomplish this with the added workflow_dispatch by manually triggering tests to run on their branch.
  2. Split sanitizers out from test-build.yml into a new test-sanitizers.yml. This new workflow doesn't automatically run on PRs, but again can be triggered with workflow_dispatch. This workflow is currently set to run on all pushes to main, but may in future be changed to run only on trigger, with the version bump triggering it to run, limiting to a max of once per day.
  3. Only cancel outdated commits if they aren't on main.

If applicable, please suggest to reviewers how they can test the change.

In light of problems with concurrent skipping, disable it.
Instead, limit the `push` trigger to just main, and enable `workflow_dispatch` for manual triggering.
Don't cancel builds from main if a new commit is pushed.
@mmicko mmicko merged commit 1a52a71 into main Aug 18, 2025
28 checks passed
@mmicko mmicko deleted the krys/ci_changes branch August 18, 2025 08:17
@widlarizer
Copy link
Collaborator

Looks like something broke on main with this

@jix jix restored the krys/ci_changes branch August 18, 2025 19:50
@mmicko mmicko deleted the krys/ci_changes branch August 25, 2025 05:49
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.

4 participants