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
Docker build cleanup #648
Docker build cleanup #648
Conversation
I think the main drawback we have here js that now we are sending the whole root dir as a context before building the image: |
Codecov Report
@@ Coverage Diff @@
## main #648 +/- ##
==========================================
+ Coverage 97.50% 97.56% +0.05%
==========================================
Files 18 18
Lines 1685 1685
==========================================
+ Hits 1643 1644 +1
+ Misses 42 41 -1
Continue to review full report at Codecov.
|
Good point. Maybe do both then? COPY and dockerignore? |
Yes. Just to be sure, we still won't |
.husky | ||
examples | ||
benchmarks | ||
# Ignore everything but the stuff following the `*` with the `!` |
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 opted for a "exclude everything" by default. It keeps the list shorter.
"prebuild-wasm": "docker build -t llhttp_wasm_builder -f build/Dockerfile .", | ||
"build-wasm": "node build/wasm.js --docker", | ||
"prebuild:wasm": "docker build -t llhttp_wasm_builder -f build/Dockerfile .", | ||
"build:wasm": "node build/wasm.js --docker", |
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 took the liberty of renaming the scripts to make them more consistent.
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.
lgtm
Remove
.dokerignore
file and use consistent naming in npm scripts.Fixes #638 .