From bd9ad10ec2e747c75a477a18d6236e12c846930f Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Thu, 7 Dec 2023 11:50:35 -0600 Subject: [PATCH] Run `semver-checks` on PRs submitted by external contributors (#3291) ## Motivation and Context Enables `cargo semver-checks` in CI for PRs created by external contributors ## Description For instance, we skipped a run of `cargo semver-checks` in https://github.com/smithy-lang/smithy-rs/pull/3286 and failed to detect [a breaking change](https://github.com/smithy-lang/smithy-rs/pull/3286#discussion_r1416479632) programmatically. With this PR, the workflow will run a job `semver-checks` even if the preceding jobs `save-docker-login-token` or `acquire-base-image` are skipped. Those jobs are relevant when the PR made changes to build tools, which is less likely for PRs created by external contributors, so it's reasonable to skip them and still run the `semver-checks` job. Furthermore, this PR enables `semver-checks` to run against all crates in `tmp-codegen-diff/aws-sdk/sdk/`, not just those limited by `list(os.listdir())[:10]`. ## Testing Tested the change against [a dummy PR](https://github.com/smithy-lang/smithy-rs/pull/3288) I created from my fork of `smithy-rs`. Specifically, `semver-checks` [caught the aforementioned breaking change](https://github.com/smithy-lang/smithy-rs/actions/runs/7121830175/job/19391798131#step:4:681) in CI. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .github/workflows/ci-pr.yml | 6 ++++++ tools/ci-scripts/codegen-diff/semver-checks.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-pr.yml b/.github/workflows/ci-pr.yml index 6e7b00603..2635554df 100644 --- a/.github/workflows/ci-pr.yml +++ b/.github/workflows/ci-pr.yml @@ -114,6 +114,12 @@ jobs: needs: - save-docker-login-token - acquire-base-image + # We need `always` here otherwise this job won't run if the previous job has been skipped + # See https://samanpavel.medium.com/github-actions-conditional-job-execution-e6aa363d2867 + if: | + always() && + !contains(needs.*.result, 'failure') && + !contains(needs.*.result, 'cancelled') steps: - uses: actions/checkout@v3 with: diff --git a/tools/ci-scripts/codegen-diff/semver-checks.py b/tools/ci-scripts/codegen-diff/semver-checks.py index 1632242fc..855cb4f2f 100755 --- a/tools/ci-scripts/codegen-diff/semver-checks.py +++ b/tools/ci-scripts/codegen-diff/semver-checks.py @@ -37,7 +37,7 @@ def main(skip_generation=False): deny_list = [ # add crate names here to exclude them from the semver checks ] - for path in list(os.listdir())[:10]: + for path in os.listdir(): eprint(f'checking {path}...', end='') if path in deny_list: eprint(f"skipping {path} because it is in 'deny_list'") -- GitLab