Repository navigation
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
cmainas
left a comment
There was a problem hiding this comment.
Hello @Inuth0603 ,
thank you for the fix. We should also add a .dockerignore file. However, do not ignore .git, since it is needed for the version of urunc. Some files and directories to ignore:
- vendor
- dist
- docs
- .github
- tests
and other non-build files in the top directory.
At last, there are some conflicts with .github/linters/urunc-dict.txt
8d9b0c9 to
108c6ac
Compare
|
Hi @cmainas, thanks for the review. I've added the Let me know if anything else needs fixing. |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @Inuth0603 for the changes. However, my suggestion about the ignore files was wrong and when I used the dockerifle to build urunc I got a -dirty suffix in the version.
So some more changes are needed:
- tests/ directory should not get ignored because the
go.modfile changes. - We should use
build-argsfor the commit sha. So in the action we need to calculate the first 7 characters form the commit sha and then pass it as build-args somehting like
``
build-args: |
COMMIT=${GITHUB_SHA::7}
| !/deployment/urunc-deploy | ||
| !/deployment/urunc-deploy/config.toml | ||
| !/deployment/urunc-deploy/scripts | ||
| !/deployment/urunc-deploy/scripts/install.sh |
There was a problem hiding this comment.
No reason for these directories if we include /deployment.
e4ba1a8 to
7d883ba
Compare
|
Hi @cmainas, thanks for the follow-up review. I've addressed the feedback and rebased onto
Let me know if anything else needs fixing. |
cmainas
left a comment
There was a problem hiding this comment.
Hello @Inuth0603 ,
please sign your commits and the new changes have introduced some new issues.
|
|
||
| # Versioning variables | ||
| COMMIT := $(shell git describe --dirty --long --always) | ||
| COMMIT ?= $(shell git describe --dirty --long --always) |
There was a problem hiding this comment.
We should keep :=. Now outside of the action a docker build without a build-args becomes an empty string for the commit sha in the version.
| REPO=${{ github.repository }} | ||
| provenance: false | ||
| build-args: | | ||
| COMMIT=${{ github.sha }} |
There was a problem hiding this comment.
We just need the first 7 characters, not the entire sha.
7d883ba to
9c05d12
Compare
|
Hi @cmainas, thanks for the review. I've made the changes and force-pushed the updates.
|
cmainas
left a comment
There was a problem hiding this comment.
Thank you @Inuth0603 for the updates. Please squash your commits
41d5c76 to
684e636
Compare
Update the urunc-deploy Docker build to use the source checked out by CI
instead of cloning the repository during the build.
- Use COPY . . in the Dockerfile and set build context to repo root
- Remove unused REPO and BRANCH build args; add COMMIT build arg
- Extract first 7 chars of GITHUB_SHA in workflow and pass as build arg
- Add .dockerignore to limit build context (exclude vendor, dist, docs, etc.)
- Keep COMMIT := in Makefile; override via RUN make COMMIT=${COMMIT} in Dockerfile
- Add contributor entry to .github/contributors.yaml
- Update spellcheck dictionary
Fixes urunc-dev#1096
Signed-off-by: Divyam Agarwal <iitdivyamagarwal@gmail.com>
684e636 to
a551777
Compare
|
Hi @cmainas, sorry for the multiple force-pushes. I had to fix the DCO sign-off and rebase onto the latest |
Update the
urunc-deployDocker build to use the source checked out by CI instead of cloning the repository during the build.Changes
COPY . .in the Dockerfile and update file paths for the repository root build context.build-latest.yml.REPOandBRANCHbuild arguments..github/contributors.yamland update spellcheck dictionary.Related issues
How was this tested?
LLM usage
Google Gemini (via Antigravity) assisted with identifying file paths, formatting, and preparing the patch. All changes were reviewed by the contributor.
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).