nealrichardson commented on a change in pull request #8824:
URL: https://github.com/apache/arrow/pull/8824#discussion_r538745060



##########
File path: .github/workflows/cpp.yml
##########
@@ -21,7 +21,7 @@ on:
   push:
     paths:
       - '.github/workflows/cpp.yml'
-      - 'ci/docker/**'
+      - 'ci/docker/*cpp*'

Review comment:
       I don't think that's correct but maybe @kszucs can weigh in 
definitively. My understanding of how docker works (which may be wrong) is that 
if image `Y` is based dockerfile `y`, and `y` is changed, then `Y` gets rebuilt 
locally in `docker-compose build Y`. If `y` is based on image `X` (and 
therefore dockerfile `x`), and `x` is changed, building `Y` will first build 
`X` and then proceed using `y` to build `Y`.  
   
   I see a few options: 
   
   1. Empirically determine which is correct
   2. Defensively, make the change I suggested (add cpp dockerfile triggers to 
ruby, python, and r workflows) and we're safe either way
   3. Make no additional change, as you propose, and risk merging changes to 
C++ dockerfiles that break downstream jobs that we'll only see after merging. 
   
   The risks entailed by #3 are probably low--these files change infrequently, 
and unless we're doing a wholesale refactor of docker images, they're probably 
low risk changes. At the same time, the cost of doing #2 is also very low and 
prevents the surprise failures that #3 could later cause. Given the low costs 
(and my hesitance to make definitive statements about how docker/docker-compose 
handles dependent dockerfiles), I still advocate for option #2. 




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to