-
-
Notifications
You must be signed in to change notification settings - Fork 14
Add builds for PRs #20
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
base: main
Are you sure you want to change the base?
Conversation
|
Some commits from #19 have leaked into this PR. |
imbev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready after #19 is merged
|
I don't think it is useful to run for each push as well. |
|
@yuravk removed the push |
|
I think this will also update the |
|
I can change it to not push when its a pull_request |
Yes, it is better to not push to registries, but just build. |
…onditions for login and push steps
|
Fixed |
|
I think |
| if: ${{ env.res != 0 || github.event_name == 'workflow_dispatch' }} | ||
| shell: bash | ||
| run: sudo podman run --rm -ti ${{ env.IMAGE_NAME }} bootc --version | ||
| run: sudo podman run --rm -ti ${{ env.IMAGE_NAME }} bootc container lint --fatal-warnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure about the use of --fatal-warnings if it will cause things to fail that should pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Containerfiles already contain linting, so that can be removed here or there.
Our disregard of the lint warnings hasn't caused major problems yet, so I think we can keep it non-fatal for now. Eventually it may be useful to enable the fatal warnings.
|
Heh. Was starting to re-implement this as I needed it, and didn't realize someone had started it already. Could we get this merged soon please? |
This will allow maintainers to test if PRs are building successfully before merging