-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Improve Dockerfile #1863
Improve Dockerfile #1863
Conversation
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.
Are there any more docker-related changes you plan to make?
.travis.yml
Outdated
- '[ -z "$WITHOUT_CURL" ] || sudo apt-get remove curl -y' | ||
script: | ||
- if [ -n "${MAKE_RELEASE-}" ]; then export GIT_EDITOR="sed -i '1 s/^/99.99.99 make release test/'" && git fetch --unshallow --tags && echo proceed | make TAG=99.99.99 release ; fi | ||
- if [ -n "${DOCTOCCHECK-}" ]; then cp README.md README.md.orig && npm run doctoc && diff -q README.md README.md.orig ; fi | ||
- if [ -n "${ECLINT-}" ]; then npm run eclint ; fi | ||
- if [ -n "${DOCKERFILE_LINT-}" ]; then npx dockerfile_lint ; fi |
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.
npx should not be used in scripts; if we want to depend on something, it should be a dependency. since this is a dependency, we should set up an npm run-script.
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.
Okay, updated.
`sudo` may lead to unpredictable behavior in some cases, and we don't really need to use `sudo` to reach what we need.
3c89157
to
4bd99bc
Compare
No more, as test integrated now. |
@ljharb anything else I need to do here? Thanks. |
No description provided.