You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/03/10 20:08:23 UTC

[GitHub] [arrow] kou commented on a diff in pull request #34512: MINOR: [CI] Self-hosted ARM workflows improvements

kou commented on code in PR #34512:
URL: https://github.com/apache/arrow/pull/34512#discussion_r1132827090


##########
.github/workflows/go.yml:
##########
@@ -42,20 +42,31 @@ permissions:
 jobs:
 
   docker:
-    name: AMD64 Debian 11 Go ${{ matrix.go }}
-    runs-on: ubuntu-latest
+    name: ${{ matrix.arch-label }} Debian 11 Go ${{ matrix.go }}
+    runs-on: ${{ matrix.runs-on }}
     if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
     timeout-minutes: 60
     strategy:
       fail-fast: false
       matrix:
-        go: [1.17, 1.18]
         include:
-          - go: 1.17
+          - arch-label: AMD64
+            arch: amd64
+            go: 1.17
+            runs-on: ubuntu-latest
             staticcheck: v0.2.2
-          - go: 1.18
+          - arch-label: AMD64
+            arch: amd64
+            go: 1.18
+            runs-on: ubuntu-latest
+            staticcheck: v0.3.3
+          - arch-label: ARM64
+            arch: arm64v8
+            go: 1.17
             staticcheck: v0.3.3

Review Comment:
   Could you use `v0.2.2` for Go 1.17 like the existing 1.17 job?
   
   ```suggestion
               staticcheck: v0.2.2
   ```
   
   Could you also remove needless code?
   
   ```diff
   diff --git a/ci/scripts/go_build.sh b/ci/scripts/go_build.sh
   index c113bbd320..3c8cc0f4ee 100755
   --- a/ci/scripts/go_build.sh
   +++ b/ci/scripts/go_build.sh
   @@ -20,13 +20,6 @@
    set -ex
    
    source_dir=${1}/go
   -ARCH=`uname -m`
   -
   -# Arm64 CI is triggered by travis and run in arm64v8/golang:1.17-bullseye
   -if [ "aarch64" == "$ARCH" ]; then
   -# Install `staticcheck`
   -  GO111MODULE=on go install honnef.co/go/tools/cmd/staticcheck@v0.2.2
   -fi
    
    pushd ${source_dir}/arrow
    
   ```



##########
.github/workflows/cpp.yml:
##########
@@ -401,15 +401,14 @@ jobs:
           PATH="$(cygpath --unix ${PYTHON_BIN_DIR}):${PATH}"
           ci/scripts/cpp_test.sh "$(pwd)" "$(pwd)/build"
 
-  linux-arm:
+  docker-arm:

Review Comment:
   Can we unify this to the existing `docker` job like Go's one?
   



-- 
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-unsubscribe@arrow.apache.org

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