Action size: Add a PR check that comments on significant repo size changes #3910
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,10 @@ | |
| runs-on: ${{ matrix.os }} | ||
| timeout-minutes: 45 | ||
|
|
||
| concurrency: | ||
| cancel-in-progress: ${{ github.event_name == 'pull_request' || false }} | ||
| group: pr-checks-unit-tests-${{ github.ref }}-${{ github.event_name }}-${{ matrix.os }}-node${{ matrix['node-version'] }} | ||
|
|
||
| steps: | ||
| - name: Prepare git (Windows) | ||
| if: runner.os == 'Windows' | ||
|
|
@@ -67,22 +71,21 @@ | |
| sarif_file: eslint.sarif | ||
| category: eslint | ||
|
|
||
| # These checks do not need to be run as part of the same matrix that we use for the `unit-tests` | ||
| # job. | ||
| other-checks: | ||
| name: Other checks | ||
| if: github.triggering_actor != 'dependabot[bot]' | ||
| permissions: | ||
| contents: read | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
|
|
||
| concurrency: | ||
| cancel-in-progress: ${{ github.event_name == 'pull_request' || false }} | ||
| group: pr-checks-pr-checks-${{ github.ref }}-${{ github.event_name }} | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
|
|
||
|
|
@@ -93,57 +96,134 @@ | |
| cache: 'npm' | ||
|
|
||
| - name: Install dependencies | ||
| id: install-deps | ||
| run: npm ci | ||
|
|
||
| - name: Verify PR checks up to date | ||
| if: ${{ !cancelled() && steps.install-deps.outcome == 'success' }} | ||
| run: .github/workflows/script/verify-pr-checks.sh | ||
|
|
||
| - name: Run pr-checks tests | ||
| if: ${{ !cancelled() && steps.install-deps.outcome == 'success' }} | ||
| working-directory: pr-checks | ||
| run: npx tsx --test | ||
|
|
||
| - name: Verify all Actions use the same Node version | ||
|
Member
There was a problem hiding this comment. This should probably also run even if the previous two steps have failed. |
||
| id: head-version | ||
| run: | | ||
| NODE_VERSION=$(find . -path "*/node_modules" -prune -o -name "action.yml" -exec yq -o=json '.runs.using' {} \; | jq -rs '[.[] | select(. != null and startswith("node"))] | unique | .[]') | ||
|
Member
There was a problem hiding this comment. I am not keen on the complexity here. I would prefer to have this kind of check implemented elsewhere and have drafted #3917 which adds an esbuild plugin for the same task. We could alternative have a script in |
||
| echo "NODE_VERSION: ${NODE_VERSION}" | ||
| if [[ $(echo "$NODE_VERSION" | wc -l) -gt 1 ]]; then | ||
| echo "::error::More than one node version used in 'action.yml' files." | ||
| exit 1 | ||
| fi | ||
| echo "node_version=${NODE_VERSION}" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Fetch base commit | ||
| id: fetch-base | ||
| # Forks and Dependabot PRs don't have permission to write comments, so skip the repo size | ||
| # check in those cases. | ||
| if: >- | ||
| github.event_name == 'pull_request' && | ||
| github.event.pull_request.head.repo.full_name == github.repository && | ||
| github.event.pull_request.user.login != 'dependabot[bot]' | ||
| env: | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | ||
|
Member
There was a problem hiding this comment. I assume that, if a PR is opened against
Contributor
Author
There was a problem hiding this comment. I was thinking the PR branch base would be preferable, so that we compare just the commits in the PR, rather than comparing against size increases or decreases on
Member
There was a problem hiding this comment. Perhaps we should do both. I see your point about comparing the size difference to the PR branch's starting point, but I could also see a case where e.g. we reduce the size of the repo on
Contributor
Author
There was a problem hiding this comment. This seems like an edge case — I think I'd rather catch this at release time than introduce more complexity here. Or if you feel strongly about this, we can look at this as followup. |
||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| # Compare against the merge base so the size delta reflects only the commits actually | ||
| # added by this PR, ignoring any changes that have landed on the base branch since the | ||
| # PR branched off. | ||
| merge_base=$(gh api "repos/$GITHUB_REPOSITORY/compare/$BASE_SHA...$HEAD_SHA" --jq '.merge_base_commit.sha') | ||
| echo "merge_base=$merge_base" >> "$GITHUB_OUTPUT" | ||
| git fetch --no-tags --depth=1 origin "$merge_base" "$HEAD_SHA" | ||
|
|
||
| - name: Check repo size | ||
Check warningCode scanning / CodeQL Checkout of untrusted code in trusted context Medium |
||
|
Comment on lines
+122
to
+142
|
||
| if: steps.fetch-base.outcome == 'success' | ||
| working-directory: pr-checks | ||
| env: | ||
| BASE_REF: ${{ github.event.pull_request.base.ref }} | ||
| BASE_SHA: ${{ steps.fetch-base.outputs.merge_base }} | ||
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | ||
| RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} | ||
| run: npx tsx check-repo-size.ts --output-dir "$RUNNER_TEMP/repo-size" | ||
|
|
||
| - name: Upload repo size comment | ||
| if: steps.fetch-base.outcome == 'success' | ||
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: repo-size-comment | ||
| path: ${{ runner.temp }}/repo-size/ | ||
| if-no-files-found: error | ||
|
|
||
| - name: 'Backport: Check out base ref' | ||
| id: checkout-base | ||
| if: ${{ startsWith(github.head_ref, 'backport-') }} | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ github.base_ref }} | ||
|
|
||
| - name: 'Backport: Verify Node versions unchanged' | ||
| if: steps.checkout-base.outcome == 'success' | ||
| env: | ||
| HEAD_VERSION: ${{ steps.head-version.outputs.node_version }} | ||
| run: | | ||
| BASE_VERSION=$(find . -path "*/node_modules" -prune -o -name "action.yml" -exec yq -o=json '.runs.using' {} \; | jq -rs '[.[] | select(. != null and startswith("node"))] | unique | .[]') | ||
| echo "HEAD_VERSION: ${HEAD_VERSION}" | ||
| echo "BASE_VERSION: ${BASE_VERSION}" | ||
| if [[ "$BASE_VERSION" != "$HEAD_VERSION" ]]; then | ||
| echo "::error::Cannot change the Node version of an Action in a backport PR." | ||
| exit 1 | ||
| fi | ||
|
|
||
| post-repo-size-comment: | ||
|
Member
There was a problem hiding this comment. We have discussed elsewhere whether this is a useful addition or not.
Contributor
Author
There was a problem hiding this comment. I'll keep it unless you feel strongly. |
||
| name: Post repo size comment | ||
| needs: other-checks | ||
| # Keep write permissions isolated from the job that checks out and tests PR code. This job only | ||
| # posts the candidate comment body produced by the read-only `pr-checks` job. | ||
| if: >- | ||
| github.event_name == 'pull_request' && | ||
| github.event.pull_request.head.repo.full_name == github.repository && | ||
| github.event.pull_request.user.login != 'dependabot[bot]' && | ||
| needs.other-checks.result == 'success' | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| runs-on: ubuntu-slim | ||
| timeout-minutes: 10 | ||
|
|
||
| concurrency: | ||
| cancel-in-progress: true | ||
| group: check-repo-size-${{ github.event.pull_request.number }} | ||
|
|
||
| steps: | ||
| - name: Download repo size comment | ||
| uses: actions/download-artifact@v8 | ||
| with: | ||
| name: repo-size-comment | ||
| path: repo-size-comment | ||
|
|
||
| - name: Post repo size comment | ||
| env: | ||
| COMMENT_MARKER: "<!-- repo-size-diff-bot -->" | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| run: | | ||
| significant=$(jq -r '.significant' repo-size-comment/metadata.json) | ||
| comment_id=$( | ||
| gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ | ||
| --paginate \ | ||
| --jq ".[] | select(.body | contains(\"$COMMENT_MARKER\")) | .id" \ | ||
| | head -n 1 | ||
| ) | ||
|
|
||
| if [[ -n "$comment_id" ]]; then | ||
| echo "Updating existing comment $comment_id." | ||
| gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$comment_id" --field body=@repo-size-comment/body.md | ||
| elif [[ "$significant" == "true" ]]; then | ||
| echo "Creating new repo size comment." | ||
| gh api --method POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" --field body=@repo-size-comment/body.md | ||
| else | ||
| echo "Skipping repo size comment because the delta is below the threshold and no sticky comment exists." | ||
| fi | ||
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.
I have a slight preference for trying to run this if
verify-pr-checks.shfails, since they perform different checks and it would be useful to know if both are failing for a PR rather than seeing theverify-pr-checks.shfailure, fixing that, only to then get a failure here. Perhaps instead of just running italways()we can check that at leastnpm ciwas successful?