Nickez/build container in ci #1278
Conversation
NickeZ
force-pushed
the
nickez/build-container-in-ci
branch
9 times, most recently
from
94011fc to
7ff7e70
Compare
There was a problem hiding this comment.
Reviewed the commits except the main commits to build the image in CI, I hope I'll get to them tomorrow.
Generally, please add a quick comment explaining the "why" to every commit message body so it's easier to review and understand.
E.g.
ci: Rename TARGET_BRANCH to COMPARE_REV
Or
ci: Use github checkout action to pull from git repo
(in this one I am not sure how the commit title relates to the changes in the commit)
NickeZ
force-pushed
the
nickez/build-container-in-ci
branch
7 times, most recently
from
455466e to
d5e56ef
Compare
NickeZ
force-pushed
the
nickez/build-container-in-ci
branch
7 times, most recently
from
7d8f213 to
818037d
Compare
|
this is ready for review again |
| * The docker image is rebuilt if the `Dockerfile` or `.containerversion` file is modified. (In case | ||
| of a push event it is also automatically published to docker hub). |
There was a problem hiding this comment.
What happens in the PR CI that modifies the Dockerfile? Technically the CI should run all commits starting from the one that modifies the Dockerfile against the new image, and the ones before against the old image.
Often Dockerfile modifications require other changes too, like fixing new clippy linters when updating the Rust toolchain, etc, and one wants to see if the PR passes CI with the new image.
There was a problem hiding this comment.
Yes, this is exactly how it works. See my test here: NickeZ#3
NickeZ
force-pushed
the
nickez/build-container-in-ci
branch
6 times, most recently
from
7237b56 to
0a04e1a
Compare
Since github will take the definition of the workflow from the merge commit, we need to take the .ci scripts from there as well. If we check out the .ci scripts from the current commit it might not match the workflow definition.
Instead of running `git fetch` in a separate script, we specify `fetch-depth: 0` in the github workflow.
When we list modified files we should only check the commits that are being added or are under review. Therefore we should use the format `git diff <commit> <commit>`. Using `git diff <commit>` will diff <commit> against the current working tree, which is only correct if the working tree is pristine. TARGET_BRANCH doesn't have to be limited to being a branch, it can be any kind of git revision. Finally instead of hard coding `master`, we use the `event.before` which has the tip of the branch before the push event.
* Check that container builds on PR * Publish container from master branch
NickeZ
force-pushed
the
nickez/build-container-in-ci
branch
from
0a04e1a to
51bc17b
Compare
Build container in CI and other cleanup regarding ci