-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat/37 client docker file #115
Conversation
f3f930e
to
7b11d60
Compare
7b11d60
to
cdeb46e
Compare
client/Dockerfile
Outdated
|
||
RUN yarn install --frozen-lockfile --production=false && \ | ||
yarn build && \ | ||
yarn install --frozen-lockfile --production=true && \ |
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.
What is the reasoning behind running yarn install with production set to false, and then to true?
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 was kind of wondering the same, it's setup the same way in the registration app.
I'm assuming it's to ensure the development build works as well? Maybe @dleard or @andrea-williams has context to give us
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.
Unless their docs are off, the only difference is that with production=false
our dev dependencies will be installed. In that case the second install won't do anything. If we need something from our dev dependencies for the build to happen well, we could consider removing the node_modules
after the yarn build
and then running the install in production mode.
Definitely not a huge deal, I was just curious.
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'd say - let's remove the whole dev statement. There is no need to ship a production image with dev dependencies, this might increase the attack surface if those deps have vulnerabilities?
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 agree, and if nothing else we'll shrink our image a little bit, which feels like a win win to me!
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'm afraid I have no insight into why the Dockerfile is written that way. It's a question for either Dylan or @Sepehr-Sobhani
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.
This seems to be a leftover from copying and pasting the Dockerfile from CIF.
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.
Looks good to me!
No description provided.