Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A better solution for skipping CI steps for docs-only changes. #16053

Merged
merged 1 commit into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/audit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

jobs:
audit:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ${{ }} are unnecessary in an if - GHA always evaluates the contents of if as an expression - and removing them makes it easier to add other conditions later by string manipulation.

runs-on: ubuntu-latest
steps:
- name: Check out code
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/cancel.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

jobs:
cancel:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
runs-on: ubuntu-latest
steps:
- uses: styfle/cancel-workflow-action@0.9.1
Expand Down
20 changes: 10 additions & 10 deletions .github/workflows/test-cron.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Bootstrap Pants, test and lint Rust (Linux-x86_64)
runs-on:
- ubuntu-20.04
Expand Down Expand Up @@ -153,7 +153,7 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Bootstrap Pants, test Rust (macOS10-x86_64)
runs-on:
- macos-10.15
Expand Down Expand Up @@ -286,7 +286,7 @@ jobs:
ARCHFLAGS: -arch arm64
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Bootstrap Pants, build wheels and fs_util (macOS11-ARM64)
runs-on:
- self-hosted
Expand Down Expand Up @@ -402,7 +402,7 @@ jobs:

src/python/pants/engine/internals/native_engine.so.metadata'
- if: (github.event_name == 'push' || !contains(env.COMMIT_MESSAGE, '[ci skip-build-wheels]'))
&& (${{ github.repository_owner == 'pantsbuild' }})
&& (github.repository_owner == 'pantsbuild')
name: Build wheels
run: USE_PY39=true arch -arm64 ./build-support/bin/release.sh build-wheels
- if: github.event_name == 'push'
Expand All @@ -420,7 +420,7 @@ jobs:
- '3.9'
timeout-minutes: 60
check_labels:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Ensure PR has a category label
runs-on:
- ubuntu-20.04
Expand All @@ -436,7 +436,7 @@ jobs:
change, category:performance, category:bugfix, category:documentation, category:internal
mode: exactly
lint_python:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Lint Python and Shell
needs: bootstrap_pants_linux_x86_64
runs-on:
Expand Down Expand Up @@ -503,7 +503,7 @@ jobs:
- '3.9'
timeout-minutes: 30
test_python_linux_x86_64_0:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Test Python (Linux-x86_64) Shard 0/3
needs: bootstrap_pants_linux_x86_64
runs-on:
Expand Down Expand Up @@ -591,7 +591,7 @@ jobs:
- '3.9'
timeout-minutes: 90
test_python_linux_x86_64_1:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Test Python (Linux-x86_64) Shard 1/3
needs: bootstrap_pants_linux_x86_64
runs-on:
Expand Down Expand Up @@ -679,7 +679,7 @@ jobs:
- '3.9'
timeout-minutes: 90
test_python_linux_x86_64_2:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Test Python (Linux-x86_64) Shard 2/3
needs: bootstrap_pants_linux_x86_64
runs-on:
Expand Down Expand Up @@ -769,7 +769,7 @@ jobs:
test_python_macos_x86_64:
env:
ARCHFLAGS: -arch x86_64
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Test Python (macOS10-x86_64)
needs: bootstrap_pants_macos_x86_64
runs-on:
Expand Down
69 changes: 35 additions & 34 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this new change, we only set the output at all if this is in fact a docs-only change. In other cases we get an empty string. So rather than setting it to "1" or "true", or something that would imply that "0" or "false" are also possible values, we set it to some sentinel string, and we check the inverse with != <the value> instead of == <the inverse value>

name: Bootstrap Pants, test and lint Rust (Linux-x86_64)
needs:
- check_labels
Expand Down Expand Up @@ -156,8 +156,8 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Bootstrap Pants, test Rust (macOS10-x86_64)
needs:
- check_labels
Expand Down Expand Up @@ -292,8 +292,8 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Build wheels and fs_util (Linux-x86_64)
needs:
- check_labels
Expand Down Expand Up @@ -378,8 +378,8 @@ jobs:
ARCHFLAGS: -arch arm64
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Bootstrap Pants, build wheels and fs_util (macOS11-ARM64)
needs:
- check_labels
Expand Down Expand Up @@ -498,7 +498,7 @@ jobs:

src/python/pants/engine/internals/native_engine.so.metadata'
- if: (github.event_name == 'push' || !contains(env.COMMIT_MESSAGE, '[ci skip-build-wheels]'))
&& (${{ github.repository_owner == 'pantsbuild' }})
&& (github.repository_owner == 'pantsbuild')
name: Build wheels
run: USE_PY39=true arch -arm64 ./build-support/bin/release.sh build-wheels
- if: github.event_name == 'push'
Expand All @@ -519,8 +519,8 @@ jobs:
env:
PANTS_REMOTE_CACHE_READ: 'false'
PANTS_REMOTE_CACHE_WRITE: 'false'
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Build wheels and fs_util (macOS10-x86_64)
needs:
- check_labels
Expand Down Expand Up @@ -619,7 +619,7 @@ jobs:
run: ./build-support/bin/deploy_to_s3.py
timeout-minutes: 80
check_labels:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Ensure PR has a category label
runs-on:
- ubuntu-20.04
Expand All @@ -635,29 +635,30 @@ jobs:
change, category:performance, category:bugfix, category:documentation, category:internal
mode: exactly
docs_only_check:
if: ${{ github.repository_owner == 'pantsbuild' }}
if: github.repository_owner == 'pantsbuild'
name: Check for docs-only change
outputs:
docs_only: ${{ steps.docs_only_check.outputs.docs_only }}
runs-on:
- ubuntu-20.04
steps:
- continue-on-error: true
id: files
- name: Check out code
uses: actions/checkout@v3
with:
fetch-depth: 10
- id: files
name: Get changed files
uses: jitterbit/get-changed-files@v1
uses: tj-actions/changed-files@v23.1
with:
format: json
files_ignore: docs/**
files_ignore_separator: '|'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is future-proofing, in case we have other paths to ignore (see the generator script for details).

- id: docs_only_check
if: steps.files.outputs.any_changed != 'true'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how the marketplace action indicates that no files in the set we care about (everything not under docs/) were changed

name: Check for docs-only changes
run: "readarray -t all_files <<<\"$(jq -r '.[]' <<<'${{ steps.files.outputs.all\
\ }}')\"\nDOCS_ONLY=1\nfor file in ${all_files[@]}; do\n if [[ ${file}\
\ != docs/* ]]; then\n DOCS_ONLY=0\n fi\ndone\nif [[ ${DOCS_ONLY}\
\ == 1 ]]; then\n echo \"::set-output name=docs_only::1\"\nelse\n echo\
\ \"::set-output name=docs_only::0\"\nfi\n"
run: echo '::set-output name=docs_only::DOCS_ONLY'
lint_python:
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Lint Python and Shell
needs:
- bootstrap_pants_linux_x86_64
Expand Down Expand Up @@ -726,7 +727,7 @@ jobs:
- '3.7'
timeout-minutes: 30
merge_ok_docs_only:
if: needs.docs_only_check.outputs.docs_only == 1
if: needs.docs_only_check.outputs.docs_only == 'DOCS_ONLY'
name: Merge OK
needs:
- docs_only_check
Expand All @@ -735,7 +736,7 @@ jobs:
steps:
- run: echo 'Merge OK'
merge_ok_not_docs_only:
if: needs.docs_only_check.outputs.docs_only == 0
if: needs.docs_only_check.outputs.docs_only != 'DOCS_ONLY'
name: Merge OK
needs:
- docs_only_check
Expand All @@ -756,8 +757,8 @@ jobs:
steps:
- run: echo 'Merge OK'
test_python_linux_x86_64_0:
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Test Python (Linux-x86_64) Shard 0/3
needs:
- bootstrap_pants_linux_x86_64
Expand Down Expand Up @@ -847,8 +848,8 @@ jobs:
- '3.7'
timeout-minutes: 90
test_python_linux_x86_64_1:
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Test Python (Linux-x86_64) Shard 1/3
needs:
- bootstrap_pants_linux_x86_64
Expand Down Expand Up @@ -938,8 +939,8 @@ jobs:
- '3.7'
timeout-minutes: 90
test_python_linux_x86_64_2:
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Test Python (Linux-x86_64) Shard 2/3
needs:
- bootstrap_pants_linux_x86_64
Expand Down Expand Up @@ -1031,8 +1032,8 @@ jobs:
test_python_macos_x86_64:
env:
ARCHFLAGS: -arch x86_64
if: (${{ github.repository_owner == 'pantsbuild' }}) && needs.docs_only_check.outputs.docs_only
== 0
if: (github.repository_owner == 'pantsbuild') && (needs.docs_only_check.outputs.docs_only
!= 'DOCS_ONLY')
name: Test Python (macOS10-x86_64)
needs:
- bootstrap_pants_macos_x86_64
Expand Down
Loading