zhztheplayer commented on code in PR #5850:
URL: https://github.com/apache/incubator-gluten/pull/5850#discussion_r1618071217


##########
backends-velox/src/test/resources/datasource/csv/student_option.csv:
##########
@@ -0,0 +1,4 @@
+Name;Language
+Juno;Java
+Peter;Python
+Celin;C++

Review Comment:
   nit: The placing folder can be renamed from `resources/datasource/csv` to 
`resource/arrow-datasource/csv` or something. 



##########
.github/workflows/velox_docker.yml:
##########
@@ -59,16 +59,24 @@ jobs:
         id: cache
         uses: actions/cache/restore@v3
         with:
-          path: ./cpp/build/releases/
+          path: |
+            ./cpp/build/releases/
+            ~/.m2/repository/org/apache/arrow/
           key: cache-velox-build-${{ hashFiles('./cache-key') }}
       - name: Build Gluten Velox third party
         if: ${{ steps.cache.outputs.cache-hit != 'true' }}
         run: |
           source dev/ci-velox-buildstatic.sh
-      - uses: actions/upload-artifact@v2
+      - name: 'Upload Artifact Native'

Review Comment:
   nit: Why quoting the name? Since we didn't do it on other names.



##########
.github/workflows/velox_docker.yml:
##########
@@ -59,16 +59,24 @@ jobs:
         id: cache
         uses: actions/cache/restore@v3
         with:
-          path: ./cpp/build/releases/
+          path: |
+            ./cpp/build/releases/
+            ~/.m2/repository/org/apache/arrow/

Review Comment:
   Should we modify velox_docker_cache.yml as well? Did you check that file 
already?



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