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

Allow PRs to build images #54

Merged
merged 27 commits into from
May 3, 2024
Merged

Allow PRs to build images #54

merged 27 commits into from
May 3, 2024

Conversation

puddly
Copy link
Collaborator

@puddly puddly commented May 1, 2024

This PR makes the container building step optional, allowing PRs that modify both to be done in two stages:

  1. Make a PR that updates Dockerfile
  2. Once merged, a new container will be built and pushed. This can then be used by subsequent PRs.

Since no write access is granted to the container for non-members, it can be used by normal PRs within CI (after approval).

@puddly puddly marked this pull request as ready for review May 1, 2024 18:20
@puddly puddly changed the title Enable PRs to build images Allow PRs to build images May 1, 2024
@puddly
Copy link
Collaborator Author

puddly commented May 2, 2024

Should be good now. Took a very long time to figure out that pushing changes to a fork triggers pull_request via CI on this repo and simultaneously push on my fork. Both now pass:

@puddly
Copy link
Collaborator Author

puddly commented May 3, 2024

I've brought build times down to about 5 minutes for the base Docker container by downloading the release ZIPs instead of checking out the SDK repos with Git. Firmwares build in about 30s each (in parallel), after 2 minutes is spent pulling the above container.

Tested with an unreleased PR bumping Gecko SDK: https://github.com/puddly/silabs-firmware-builder/actions/runs/8940084401

@puddly puddly requested a review from agners May 3, 2024 14:05
@agners
Copy link
Collaborator

agners commented May 3, 2024

Nice!

@puddly puddly merged commit ff6487b into NabuCasa:main May 3, 2024
15 checks passed
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.

2 participants