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

building only on approval saves us some cpu - do not merge #133

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Sybrand
Copy link
Collaborator

@Sybrand Sybrand commented Aug 22, 2024

Ideally images are buildable before merging - but building on every commit to a PR is too expensive.

What we do now, is wait until a PR is approved before building it.

This way branch rules can be kept in place (must be built before merging) and don't waste cycles building on each commit.

@Sybrand Sybrand requested a review from zenenock August 22, 2024 07:28
@okumujustine
Copy link

Looked at this even though it's assigned to @zenenock, it looks good.

@Sybrand Sybrand changed the title building only on approval saves us some cpu building only on approval saves us some cpu - do not merge Sep 12, 2024
@Sybrand
Copy link
Collaborator Author

Sybrand commented Sep 12, 2024

Turns out this isn't as easy as I had hoped.

pull_request_review:
      types: [submitted]

Only works for you on the approve event, you can't access github.event.review.state on any push after the approval. If we enforced that approval is required on a push after approval then it would be ok - but we don't want that kind of bureaucracy.

Also - the reference github.head_ref || github.ref_name is slightly different on an approval event.

@Sybrand Sybrand requested review from zenenock and removed request for zenenock September 12, 2024 13:39
@Sybrand Sybrand marked this pull request as draft September 12, 2024 13:40
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