This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Avoid running CI steps when the files they check have not been changed. #14745
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
aa73b23
Reduce linting, unit test and integration test runs to when the files…
reivilibre b39e175
Newsfile
reivilibre 25a5bbf
Update lists of files.
clokep b1a0099
Clarify comment
clokep d0fabd8
Merge branch 'develop' into rei/reduce_ci_uselessness
clokep 65178b8
Include docker for integration tests.
clokep 84ac943
Merge remote-tracking branch 'origin/develop' into rei/reduce_ci_usel…
clokep File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,9 @@ jobs: | |
runs-on: ubuntu-latest | ||
outputs: | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
rust: ${{ !startsWith(github.ref, 'refs/pull/') || steps.filter.outputs.rust }} | ||
trial: ${{ !startsWith(github.ref, 'refs/pull/') || steps.filter.outputs.trial }} | ||
integration: ${{ !startsWith(github.ref, 'refs/pull/') || steps.filter.outputs.integration }} | ||
linting: ${{ !startsWith(github.ref, 'refs/pull/') || steps.filter.outputs.linting }} | ||
steps: | ||
- uses: dorny/paths-filter@v2 | ||
id: filter | ||
|
@@ -33,9 +36,45 @@ jobs: | |
- 'rust/**' | ||
- 'Cargo.toml' | ||
- 'Cargo.lock' | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- '.rustfmt.toml' | ||
|
||
trial: | ||
- 'synapse/**' | ||
- 'tests/**' | ||
- 'rust/**' | ||
- 'Cargo.toml' | ||
- 'Cargo.lock' | ||
- 'pyproject.toml' | ||
- 'poetry.lock' | ||
|
||
integration: | ||
- 'synapse/**' | ||
- 'rust/**' | ||
- 'docker/**' | ||
- 'Cargo.toml' | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- 'Cargo.lock' | ||
- 'pyproject.toml' | ||
- 'poetry.lock' | ||
- 'docker/**' | ||
|
||
linting: | ||
- 'synapse/**' | ||
- 'docker/**' | ||
- 'tests/**' | ||
- 'scripts-dev/**' | ||
- 'contrib/**' | ||
- 'synmark/**' | ||
- 'stubs/**' | ||
- '.ci/**' | ||
- 'mypy.ini' | ||
- 'pyproject.toml' | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- 'poetry.lock' | ||
|
||
check-sampleconfig: | ||
runs-on: ubuntu-latest | ||
needs: changes | ||
if: ${{ needs.changes.outputs.linting == 'true' }} | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Install Rust | ||
|
@@ -51,6 +90,9 @@ jobs: | |
|
||
check-schema-delta: | ||
runs-on: ubuntu-latest | ||
needs: changes | ||
if: ${{ needs.changes.outputs.linting == 'true' }} | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v4 | ||
|
@@ -70,6 +112,9 @@ jobs: | |
|
||
lint: | ||
runs-on: ubuntu-latest | ||
needs: changes | ||
if: ${{ needs.changes.outputs.linting == 'true' }} | ||
|
||
steps: | ||
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
|
@@ -92,6 +137,9 @@ jobs: | |
lint-mypy: | ||
runs-on: ubuntu-latest | ||
name: Typechecking | ||
needs: changes | ||
if: ${{ needs.changes.outputs.linting == 'true' }} | ||
|
||
steps: | ||
- name: Checkout repository | ||
uses: actions/checkout@v4 | ||
|
@@ -149,6 +197,9 @@ jobs: | |
|
||
lint-pydantic: | ||
runs-on: ubuntu-latest | ||
needs: changes | ||
if: ${{ needs.changes.outputs.linting == 'true' }} | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
|
@@ -250,8 +301,10 @@ jobs: | |
sytest_test_matrix: ${{ steps.get-matrix.outputs.sytest_test_matrix }} | ||
|
||
trial: | ||
if: ${{ !cancelled() && !failure() }} # Allow previous steps to be skipped, but not fail | ||
needs: calculate-test-jobs | ||
if: ${{ !cancelled() && !failure() && needs.changes.outputs.trial == 'true' }} # Allow previous steps to be skipped, but not fail | ||
needs: | ||
- calculate-test-jobs | ||
- changes | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
|
@@ -306,8 +359,10 @@ jobs: | |
|
||
trial-olddeps: | ||
# Note: sqlite only; no postgres | ||
if: ${{ !cancelled() && !failure() }} # Allow previous steps to be skipped, but not fail | ||
needs: linting-done | ||
if: ${{ !cancelled() && !failure() && needs.changes.outputs.trial == 'true' }} # Allow previous steps to be skipped, but not fail | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside: I think the olddeps job doesn't care if poetry.lock changes, only if pyproject.toml does. Not worth the yaml faff to encode that here though! |
||
needs: | ||
- linting-done | ||
- changes | ||
runs-on: ubuntu-20.04 | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
@@ -362,8 +417,10 @@ jobs: | |
trial-pypy: | ||
# Very slow; only run if the branch name includes 'pypy' | ||
# Note: sqlite only; no postgres. Completely untested since poetry move. | ||
if: ${{ contains(github.ref, 'pypy') && !failure() && !cancelled() }} | ||
needs: linting-done | ||
if: ${{ contains(github.ref, 'pypy') && !failure() && !cancelled() && needs.changes.outputs.trial == 'true' }} | ||
needs: | ||
- linting-done | ||
- changes | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
|
@@ -394,8 +451,10 @@ jobs: | |
|| true | ||
|
||
sytest: | ||
if: ${{ !failure() && !cancelled() }} | ||
needs: calculate-test-jobs | ||
if: ${{ !failure() && !cancelled() && needs.changes.outputs.integration == 'true' }} | ||
needs: | ||
- calculate-test-jobs | ||
- changes | ||
runs-on: ubuntu-latest | ||
container: | ||
image: matrixdotorg/sytest-synapse:${{ matrix.job.sytest-tag }} | ||
|
@@ -476,8 +535,10 @@ jobs: | |
|
||
|
||
portdb: | ||
if: ${{ !failure() && !cancelled() }} # Allow previous steps to be skipped, but not fail | ||
needs: linting-done | ||
if: ${{ !failure() && !cancelled() && needs.changes.outputs.linting == 'true' }} # Allow previous steps to be skipped, but not fail | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
needs: | ||
- linting-done | ||
- changes | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
|
@@ -537,8 +598,10 @@ jobs: | |
schema_diff | ||
|
||
complement: | ||
if: "${{ !failure() && !cancelled() }}" | ||
needs: linting-done | ||
if: "${{ !failure() && !cancelled() && needs.changes.outputs.integration == 'true' }}" | ||
needs: | ||
- linting-done | ||
- changes | ||
runs-on: ubuntu-latest | ||
|
||
strategy: | ||
|
@@ -632,9 +695,14 @@ jobs: | |
with: | ||
needs: ${{ toJSON(needs) }} | ||
|
||
# The newsfile and signoff lints may be skipped on non PR builds | ||
# Cargo test is skipped if there is no changes on Rust code | ||
# Various bits are skipped if there was no applicable changes. | ||
# The newsfile and signoff lint may be skipped on non PR builds. | ||
skippable: | | ||
trial | ||
trial-olddeps | ||
sytest | ||
portdb | ||
complement | ||
check-signoff | ||
lint-newsfile | ||
cargo-test | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Avoid running CI steps when the files they check have not been changed. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ironically it seems that many of the steps have been skipped on this PR, I wonder if we should include
.github/workflows
in the list of steps to run most jobs?