Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak CI: remove on push for PRs and add names to CI steps #37

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

MattijsKneppers
Copy link
Contributor

@MattijsKneppers MattijsKneppers commented Jul 4, 2024

We originally included push to make sure PRs get tested when new commits are added or existing commits are changed.

However, it appears that for an active PR, every push already causes a synchronize activity type for the pull_request_target event, which triggers the workflow. See the pull_request_target documentation.

So the push event is no longer needed for PRs. Instead, we set it to trigger only for the case when something was pushed directly to the main branch as documented here.

Also adds names to CI steps like in GitHub's examples.

@jamesb93
Copy link

jamesb93 commented Jul 4, 2024

Makes sense!

@MattijsKneppers
Copy link
Contributor Author

A good point from @ala-ableton: when removing the push event, the tests will no longer run when something is pushed directly to main. To counter that I re-added push but set it to only trigger for the main branch as documented here.

We included `push` to make sure PRs get tested when new commits were added or the commit history was changed. However, it appears that for an active PR, every push already causes a `synchronize` activity type for the `pull_request_target` event.

So the `push` event is no longer needed for every PR. Instead, this sets it to trigger only for the case when something was pushed directly to `main`.

See:
https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target
@MattijsKneppers MattijsKneppers changed the title Tweak CI: remove on push and add names to CI steps Tweak CI: remove on push for PRs and add names to CI steps Jul 4, 2024
@MattijsKneppers MattijsKneppers merged commit 544f493 into main Jul 4, 2024
3 checks passed
@MattijsKneppers MattijsKneppers deleted the mkp-tweak-ci branch July 4, 2024 09:21
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