[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5298: Add ort integration testing.

[GitBox <gi...@apache.org>]

zrhoffman commented on a change in pull request #5298:
URL: https://github.com/apache/trafficcontrol/pull/5298#discussion_r564829650



##########
File path: traffic_ops_ort/testing/docker/yumserver/Dockerfile
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+############################################################
+# Dockerfile to build Traffic Server container images
+#   as Edges for Traffic Control 1.4
+# Based on CentOS 6.6
+############################################################
+
+FROM centos:7
+MAINTAINER dev@trafficcontrol.apache.org
+
+RUN yum install -y httpd
+RUN yum install -y createrepo
+RUN yum install -y yum-utils

Review comment:
       
   Installing each package in a separate layer leaders to inconsistent behavior. Depending on what modifications are made to the Dockerfile in the future, some layers will be cached and some will not, meaning that the image is no longer well-defined by a build date and a list of packages.
   
   [Dockerfile best practices](https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#apt-get) dictate grouping the list of packages to be installed into a single layer, preferably starting with a `yum -y update`, which will ensure that all packages listed are installed (see: "cache busting").
   
   > it works, this is fine.
   
   A big part of why a reviewer's input matters is that this code will eventually break and need to be maintained, and in a shared project, no contributor can claim with certainty that they will be the sole one to fix that code in the future. Requests to decrease the maintenance cost of a code fragment should be heeded.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org