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

run CI GHA on pull_request so that external contributors benefit from CI #4148

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

twrichards
Copy link
Contributor

@twrichards twrichards commented Oct 5, 2023

Following/tweaking #4146 - we do need to trigger the unified CI GHA based on pull_request* and not just 'all branches' since this doesn't include branches on forks (understandably) and I imagine our external contributors benefit from the CI checks (compilation, tests etc.) on their PRs.

To complement the above, we limit push based trigger to main branch only to avoid duplicate workflow runs (wasteful) as well as branches starting with pr (i.e. those created by dev/script/push-pr.sh).

* this does have the unfortunate downside of building the hidden pull/XXX/merge branch, which is actually a preview of what the merge would look like rather than the branch the PR has been raised from - whilst isn't normally an issue but can on occasion cause pernicious problems, so would normally avoid it - but alas its necessary here. You can see an example from this PR's workflow run...
image

@twrichards twrichards requested a review from a team as a code owner October 5, 2023 15:33
Copy link
Contributor

@Georges-GNM Georges-GNM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

…om CI and limit `push` based trigger to `main` branch only to avoid duplicate workflow runs (wasteful)
@prout-bot
Copy link

Seen on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, collections, media-api, kahuna (merged by @twrichards 12 minutes and 7 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants