Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

take Kumascript Dockerfile from kuma #1249

Merged
merged 1 commit into from
Oct 4, 2019
Merged

take Kumascript Dockerfile from kuma #1249

merged 1 commit into from
Oct 4, 2019

Conversation

escattone
Copy link
Contributor

This is a companion PR to mdn/kuma#5902. It should be reviewed and merged along with that PR.

This PR moves the Kumascript Dockerfile (and associated README.md file) from the Kuma repo into this repo where it belongs. It also takes advantage of the new make build-kumascript-with-all-tags command provided by mdn/kuma#5902.

@escattone
Copy link
Contributor Author

escattone commented Oct 1, 2019

@peterbe Do I need to create/update some settings file for renovate so that it begins updating docker/Dockerfile? Or maybe we're not even using renovate yet for this repo and I need to configure that as well?

@escattone
Copy link
Contributor Author

@peterbe Do I need to create/update some settings file for renovate so that it begins updating docker/Dockerfile? Or maybe we're not even using renovate yet for this repo and I need to configure that as well?

Ah, yeah, it looks like we're not using renovate yet in this repo. I'll look into setting it up once this lands.

RUN npm config set python /usr/bin/python2.7 && \
# install the Node.js dependencies,
# with versions specified in npm-shrinkwrap.json
npm ci && \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm ci is an alias to npm clean‑install

Suggested change
npm ci && \
npm clean-install && \

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ExE-Boss It's my understanding that npm ci is the proper command. In fact, I don't see any npm documentation on a npm clean-install command, and I think the ci references continuous integration rather than clean install. In any case, I'm going to keep this as it is, since changing it would be outside the scope of this PR anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is as of npm/cli#57.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Good to know, thanks @ExE-Boss, but I'll leave that for another PR.

@escattone escattone requested review from limed and removed request for peterbe October 1, 2019 23:06
@escattone
Copy link
Contributor Author

@peterbe I removed you as a reviewer since I realized that I was pulling your mind into this realm when you've already got so much on your plate, but of course feel free to review if you'd like to.

Copy link
Contributor

@limed limed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Copy link
Contributor

@limed limed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@escattone escattone merged commit 10f78b4 into mdn:master Oct 4, 2019
@escattone escattone deleted the move-kumascript-dockerfile branch October 4, 2019 19:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants