Skip to content

ci: Use docker action to clone repo #9

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

quic-nasserg
Copy link

We can use the docker/build-push-action with the 'context' input to clone the repo for us.

Also rename the action to reflect the primary purpose of "build" instead of "pull" and update the inputs.

We can use the docker/build-push-action with the 'context' input
to clone the repo for us.

Also rename the action to reflect the primary purpose of "build"
instead of "pull" and update the inputs.

Signed-off-by: Nasser Grainawi <[email protected]>
@quic-nasserg
Copy link
Author

quic-nasserg commented May 29, 2025

@quic-viskuma can you review please?

@quic-viskuma
Copy link
Contributor

quic-viskuma commented May 30, 2025

@quic-viskuma can you review please?

LGTM

default: kmake-image:latest
image_context:
description: The context to use for the image build. Likely a git repo URL.
required: true
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make it required false? we can use the default value for now

Copy link
Author

Choose a reason for hiding this comment

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

Using required=true is fine as I just learned that GitHub doesn't actually do any enforcement on required. But if they did ever implement it (it's been 5+ years since users asked for it...), I would expect that required=true with a default should still work.

@quic-viskuma quic-viskuma self-requested a review June 18, 2025 11:34
@quic-viskuma quic-viskuma force-pushed the main branch 5 times, most recently from dbf4b91 to df6efde Compare July 14, 2025 13:51
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