alamb commented on code in PR #16709:
URL: https://github.com/apache/datafusion/pull/16709#discussion_r2202500711


##########
.github/actions/setup-macos-aarch64-builder/action.yaml:
##########
@@ -45,5 +45,7 @@ runs:
         rustup component add rustfmt
     - name: Setup rust cache
       uses: Swatinem/rust-cache@v2

Review Comment:
   TIL that `rust-cache` caches the Rust binaries too
   
   https://github.com/Swatinem/rust-cache?tab=readme-ov-file#cache-details



##########
.github/actions/setup-macos-aarch64-builder/action.yaml:
##########
@@ -45,5 +45,7 @@ runs:
         rustup component add rustfmt
     - name: Setup rust cache
       uses: Swatinem/rust-cache@v2

Review Comment:
   Also, something I noticed was maybe we should pin to an actual hash rather 
than a tag that is updated
   
   https://github.com/Swatinem/rust-cache/tags
   
   I think security best practice would be to pin to 
https://github.com/Swatinem/rust-cache/commit/98c8021b550208e191a6a3145459bfc9fb29c4c0
 for example rather than https://github.com/Swatinem/rust-cache/releases/tag/v2
   
   However, this wasn't introduced in this PR so I don't think it needs to be 
fixed here 
   
   



##########
.github/actions/setup-macos-aarch64-builder/action.yaml:
##########
@@ -45,5 +45,7 @@ runs:
         rustup component add rustfmt
     - name: Setup rust cache
       uses: Swatinem/rust-cache@v2
+      with:
+           save-if: ${{ github.ref_name == 'main' }}

Review Comment:
   Should this also have the `shared-key: amd` ?



-- 
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: github-unsubscr...@datafusion.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to