search

redhat-actions / buildah-build

GitHub Action to use 'buildah' to build a container image.
https://github.com/marketplace/actions/buildah-build
MIT License
144 stars 34 forks source link

Add tests #11

Closed divyansh42 closed 3 years ago

divyansh42 commented 3 years ago

Changes

Closes #2 #8

tetchel commented 3 years ago

Nice work. I wonder if we could leave the push-to-registry out of this for simplicity, since that's out of scope of this repo.

Also it means we can leave the quay secrets out of this repo, and not fill up your quay.io with all these test images.

tetchel commented 3 years ago

Running this as-is against pull_request is not really helpful, because it's testing master, not the PR code.

We could change it so that the repo checked out GITHUB_REF instead, and then it would be an effective PR test.

divyansh42 commented 3 years ago

Nice work. I wonder if we could leave the push-to-registry out of this for simplicity, since that's out of scope of this repo.

Also it means we can leave the quay secrets out of this repo, and not fill up your quay.io with all these test images.

@tetchel I think removing the push-to-repository action makes sense. But I am wonderig now example will be same as what we are doing in verify-build.yaml. It will be kind of repetitive thing.

So does it makes sense to remove example in this?

divyansh42 commented 3 years ago

Running this as-is against pull_request is not really helpful, because it's testing master, not the PR code.

We could change it so that the repo checked out GITHUB_REF instead, and then it would be an effective PR test.

I have updated the PR based on your review. Please have a look.

tetchel commented 3 years ago

I agree it makes sense to remove the example since the other workflow is a better version.

In addition to the feedback above, please try to put any strings that are repeated through the workflow into environment variables, so they can be maintained more easily.