Skip to content

Commit 0b1b3fd

Browse files
authored
Improve CI workflow comments for maintainability (#1504)
Clarify why build and test jobs are split by platform rather than using a matrix: to allow independent dependencies so faster platforms can proceed through build & test without waiting for slower ones. Document the allowed textual differences between job definitions. Also unify the if: conditions across test jobs for consistency.
1 parent 4404b43 commit 0b1b3fd

1 file changed

Lines changed: 19 additions & 11 deletions

File tree

.github/workflows/ci.yml

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,12 @@ jobs:
6262
echo "skip=${skip}" >> "$GITHUB_OUTPUT"
6363
echo "doc_only=${doc_only}" >> "$GITHUB_OUTPUT"
6464
65-
# WARNING: make sure all of the build jobs are in sync
65+
# NOTE: Build jobs are intentionally split by platform rather than using a single
66+
# matrix. This allows each test job to depend only on its corresponding build,
67+
# so faster platforms can proceed through build & test without waiting for slower
68+
# ones. Keep these job definitions textually identical except for:
69+
# - host-platform value
70+
# - if: condition (build-linux-64 omits doc-only check since it's needed for docs)
6671
build-linux-64:
6772
needs:
6873
- ci-vars
@@ -81,7 +86,7 @@ jobs:
8186
cuda-version: ${{ needs.ci-vars.outputs.CUDA_BUILD_VER }}
8287
prev-cuda-version: ${{ needs.ci-vars.outputs.CUDA_PREV_BUILD_VER }}
8388

84-
# WARNING: make sure all of the build jobs are in sync
89+
# See build-linux-64 for why build jobs are split by platform.
8590
build-linux-aarch64:
8691
needs:
8792
- ci-vars
@@ -100,7 +105,7 @@ jobs:
100105
cuda-version: ${{ needs.ci-vars.outputs.CUDA_BUILD_VER }}
101106
prev-cuda-version: ${{ needs.ci-vars.outputs.CUDA_PREV_BUILD_VER }}
102107

103-
# WARNING: make sure all of the build jobs are in sync
108+
# See build-linux-64 for why build jobs are split by platform.
104109
build-windows:
105110
needs:
106111
- ci-vars
@@ -119,7 +124,11 @@ jobs:
119124
cuda-version: ${{ needs.ci-vars.outputs.CUDA_BUILD_VER }}
120125
prev-cuda-version: ${{ needs.ci-vars.outputs.CUDA_PREV_BUILD_VER }}
121126

122-
# WARNING: make sure both Linux test jobs are in sync
127+
# NOTE: Test jobs are split by platform for the same reason as build jobs (see
128+
# build-linux-64). Keep these job definitions textually identical except for:
129+
# - host-platform value
130+
# - build job under needs:
131+
# - uses: (test-wheel-linux.yml vs test-wheel-windows.yml)
123132
test-linux-64:
124133
strategy:
125134
fail-fast: false
@@ -141,21 +150,20 @@ jobs:
141150
host-platform: ${{ matrix.host-platform }}
142151
build-ctk-ver: ${{ needs.ci-vars.outputs.CUDA_BUILD_VER }}
143152

144-
# WARNING: make sure both Linux test jobs are in sync
153+
# See test-linux-64 for why test jobs are split by platform.
145154
test-linux-aarch64:
146155
strategy:
147156
fail-fast: false
148157
matrix:
149158
host-platform:
150159
- linux-aarch64
151160
name: Test ${{ matrix.host-platform }}
152-
# Note: No doc-only check needed here - if build-linux-aarch64 is skipped,
153-
# this job is automatically skipped due to the dependency.
154-
if: ${{ github.repository_owner == 'nvidia' }}
161+
if: ${{ github.repository_owner == 'nvidia' && !fromJSON(needs.should-skip.outputs.doc-only) }}
155162
permissions:
156163
contents: read # This is required for actions/checkout
157164
needs:
158165
- ci-vars
166+
- should-skip
159167
- build-linux-aarch64
160168
secrets: inherit
161169
uses: ./.github/workflows/test-wheel-linux.yml
@@ -164,20 +172,20 @@ jobs:
164172
host-platform: ${{ matrix.host-platform }}
165173
build-ctk-ver: ${{ needs.ci-vars.outputs.CUDA_BUILD_VER }}
166174

175+
# See test-linux-64 for why test jobs are split by platform.
167176
test-windows:
168177
strategy:
169178
fail-fast: false
170179
matrix:
171180
host-platform:
172181
- win-64
173182
name: Test ${{ matrix.host-platform }}
174-
# Note: No doc-only check needed here - if build-windows is skipped,
175-
# this job is automatically skipped due to the dependency.
176-
if: ${{ github.repository_owner == 'nvidia' }}
183+
if: ${{ github.repository_owner == 'nvidia' && !fromJSON(needs.should-skip.outputs.doc-only) }}
177184
permissions:
178185
contents: read # This is required for actions/checkout
179186
needs:
180187
- ci-vars
188+
- should-skip
181189
- build-windows
182190
secrets: inherit
183191
uses: ./.github/workflows/test-wheel-windows.yml

0 commit comments

Comments
 (0)