Uh oh!
There was an error while loading. Please reload this page.
update docker webapp guide for carbon - #1467
Conversation
- updates refrences of boron for carbon - updates express package version in example (4.16.1 is latest stable) - updates Dockerfile `COPY` (and instructions) to assume `npm@5+` and warn about `npm@4-` - updates various stdout examples
| COPY package.json . | ||
| # For npm@5 or later, copy package-lock.json as well | ||
| # COPY package.json package-lock.json ./ | ||
| COPY package.json package-lock.json ./ |
There was a problem hiding this comment.
I considered making this a wildcard to account for both cases... but thought this was more straight-forward and consistent what was here before.
The following will not fail if the package-lock.json file does not exist (using an older version of npm or if npm install was not run before docker build ...:
COPY package.json package-*.json ./
There was a problem hiding this comment.
At this point, COPY . should be enough IMHO. No need to explicitly copy package*.json again.
There was a problem hiding this comment.
Just to clarify, are you suggesting to forego the explicit coping of package*.json files and simply COPY . .before the npm install (removing the # Bundle app source command)? Resulting in:
FROM node:carbon
# Create app directory
WORKDIR /usr/src/app
# Bundle app source
COPY . .
# Install app dependencies
RUN npm install
# If you are building your code for production
# RUN npm install --only=production
EXPOSE 8080
CMD [ "npm", "start" ]
Or, replacing the replacing the COPY package.json package-lock.json ./ with COPY package*.json .?
There was a problem hiding this comment.
Sorry, my mistake, got a bit mixed up in the diff. I meant
replacing the replacing the
COPY package.json package-lock.json ./withCOPY package*.json .?
jbottigliero
commented
Nov 13, 2017
If this looks good, I can at the very least update the examples/stdout in the translation files if that is desired. |
| COPY package.json . | ||
| # For npm@5 or later, copy package-lock.json as well | ||
| # COPY package.json package-lock.json ./ | ||
| COPY package.json package-lock.json ./ |
There was a problem hiding this comment.
At this point, COPY . should be enough IMHO. No need to explicitly copy package*.json again.
fhemberger
commented
Nov 14, 2017
Otherwise LGTM. |
jbottigliero
commented
Nov 16, 2017
Updated to use For the additional locale versions, is it worth updating the
The above seems like it would be the only misleading copy from the original (translated) version. |
fhemberger
commented
Nov 16, 2017
@jbottigliero Sounds good to me. 👍 |
jbottigliero
commented
Nov 17, 2017
I've opened #1476 adding the updates mentioned in this comment along a few URL updates. |
COPY(and instructions) to assumenpm@5+and warn aboutnpm@4-