[Asterisk-code-review] Add initial support to build Docker images (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: Add initial support to build Docker images
......................................................................


Patch Set 1:

(4 comments)

No serious issues, mostly just nit-pick questions.

https://gerrit.asterisk.org/#/c/2297/1/contrib/docker/Dockerfile.asterisk
File contrib/docker/Dockerfile.asterisk:

Line 8: COPY $RPMPATH/*.rpm $STARTDIR
I don't understand the benefit of this.  Wouldn't this add the RPM's as an image layer? It seems like they could just be installed with 'yum install -y $RPMPATH/*.rpm'?


Line 9: RUN yum install epel-release -y
Why do this separate?  I mean I understand that it needs to be a separate call to 'yum install', but why not put it at the start of the same RUN that performs other yum commands?


https://gerrit.asterisk.org/#/c/2297/1/contrib/docker/Dockerfile.packager
File contrib/docker/Dockerfile.packager:

Line 9: RUN /tmp/install_prereq install
We should have a 'yum clean all' after running install_prereq.  Any reason we can't reorder things so the 'ADD' is first then everything in a single 'RUN'?


https://gerrit.asterisk.org/#/c/2297/1/contrib/docker/README.md
File contrib/docker/README.md:

Line 23: > you'll need to use the following command instead:
it looks the same to me.  What am I missing?


-- 
To view, visit https://gerrit.asterisk.org/2297
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifa070706d40e56755797097b6ed72c1e243bd0d1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Leif Madsen <leif at leifmadsen.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes