-
Notifications
You must be signed in to change notification settings - Fork 10
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
pull the latest version of mono instead of pinning to an old version #21
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.
Since the upstream mono image is now Stretch, this Dockerfile can be simplified quite a lot, thanks!
Dockerfile
Outdated
@@ -1,4 +1,4 @@ | |||
FROM mono:3.12.1 as builder | |||
FROM mono as builder | |||
ARG CHOCOVERSION=stable | |||
|
|||
RUN echo "deb http://archive.debian.org/debian/ wheezy main contrib non-free" >/etc/apt/sources.list |
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 don't think this line should be here; Current version of mono is based on Debian 9 stretch.
Dockerfile
Outdated
@@ -1,4 +1,4 @@ | |||
FROM mono:3.12.1 as builder | |||
FROM mono as builder |
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 think FROM mono:latest
would be more explicit, although I'm always hesitant to use "latest" for anything. How about FROM mono:6.8
, which would pick up 6.8* according to https://hub.docker.com/_/mono/
I don't think that a multi-stage build is required at all now, since both this Dockerfile and the upstream mono image are both Debian Stretch?
@rfay Thanks for those excellent points. I've given it another pass. Let me know what you think :) |
@TheCakeIsNaOH Thanks for the other PR. I'll close this in favor of that one. |
Fixes #18