kgyrtkirk commented on code in PR #15106:
URL: https://github.com/apache/druid/pull/15106#discussion_r1370257915


##########
.github/workflows/static-checks.yml:
##########
@@ -51,12 +51,33 @@ jobs:
         with:
           distribution: 'zulu'
           java-version: ${{ matrix.java }}
-          cache: 'maven'
+
+      - name: restore maven dependencies
+      # restore maven dependencies only. 'depsonly' keys are safe to restore
+      # since they don't contain any artifacts created by mvn install
+      - uses: actions/cache/restore@v3
+        with:
+          key: maven-${{ runner.os }}-depsonly-${{ hashFiles('**/pom.xml') }}
+          path: ~/.m2/repository
+          restore-keys: |
+            maven-${{ runner.os }}-depsonly-

Review Comment:
   I think this should be removed as it could result in dragging outdate 
artifacts forward:
   * project declares `artifact#1.0`
   * cache stores `artifact#1.0`
   * pom changes to use `artifact#2.0`
   * because of fallback next cache will still contain `artifact#1.0`
   



##########
.github/workflows/static-checks.yml:
##########
@@ -156,7 +185,15 @@ jobs:
         with:
           distribution: 'zulu'
           java-version: '17'
-          cache: 'maven'
+
+      # restore maven dependencies only. 'depsonly' keys are safe to restore
+      # since they don't contain any artifacts created by mvn install
+      - uses: actions/cache@v3

Review Comment:
   shouldn't this be `actions/cache/restore@v3` ? 



##########
.github/workflows/static-checks.yml:
##########
@@ -51,12 +51,33 @@ jobs:
         with:
           distribution: 'zulu'
           java-version: ${{ matrix.java }}
-          cache: 'maven'
+
+      - name: restore maven dependencies
+      # restore maven dependencies only. 'depsonly' keys are safe to restore
+      # since they don't contain any artifacts created by mvn install
+      - uses: actions/cache/restore@v3
+        with:
+          key: maven-${{ runner.os }}-depsonly-${{ hashFiles('**/pom.xml') }}
+          path: ~/.m2/repository
+          restore-keys: |
+            maven-${{ runner.os }}-depsonly-
+
+      - name: compile and cache maven dependencies
+        # run maven compile step to cache all shared maven dependencies
+        # maven install will be run in a subsequent step to avoid polluting 
the global maven cache

Review Comment:
   I wonder why you haven't replied back to my comments;
   why can't we just exclude the artifacts built by the project from the cache?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to