[GitHub] [cassandra-builds] smiklosovic commented on a change in pull request #65: Updated ant to 1.10.12 for Docker images

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

smiklosovic commented on a change in pull request #65:
URL: https://github.com/apache/cassandra-builds/pull/65#discussion_r809841685



##########
File path: docker/almalinux-image.docker
##########
@@ -20,17 +19,21 @@ RUN yum -y install \
    sudo \
    python3-pip
 
-RUN until curl -S -s --retry 9 --retry-connrefused --retry-delay 1 http://mirror.centos.org/centos/7/os/x86_64/Packages/ant-junit-1.9.4-2.el7.noarch.rpm -o ant-junit-1.9.4-2.el7.noarch.rpm ; do echo "curl failed… trying again… " ; done
-
-RUN rpm -i --nodeps ant-junit-1.9.4-2.el7.noarch.rpm
-
 RUN pip3 install --upgrade pip
 
 # install Sphinx to generate docs
 RUN pip3 install \
    Sphinx \
    sphinx_rtd_theme
 
+# install ant
+RUN mkdir /usr/share/ant && \

Review comment:
       I would
   
   1) move ENVs to the top of this docker file, I do not like when ENV's are all over the place
   2) I would make environment property out of Ant version too (ANT_VERSION=1.10.12) and I would use it in wget and tar commands. This way, you may test your image with different Ant replacing it at one place or injecting it from outside
   
   https://www.baeldung.com/ops/docker-container-environment-variables




-- 
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.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org