Skip to content

Commit

Permalink
refactor(ci/pr-review): use workflow_call instead of workflow_run…
Browse files Browse the repository at this point in the history 
… event (v2) (#30064)

* refactor(ci/pr-review): use workflow_call instead of `workflow_run` event

* prevent run PR test on forks

* fix: correct the job-id

* fix: remove the unnecessary condition

* apply suggestions

* Add note about permissions

* move the `if` condition to the top

* Update .github/workflows/pr-test.yml

Co-authored-by: Brian Thomas Smith <[email protected]>

* bump actions/setup-node from v3 to v4

* fix(pr-test): inherit secrets

---------

Co-authored-by: Brian Thomas Smith <[email protected]>
Co-authored-by: Claas Augner <[email protected]>
  • Loading branch information
@yin1999 @bsmth @caugner
3 people committed Nov 6, 2023
1 parent 0c40b9e commit 9b1c805
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 72 deletions.
50 changes: 5 additions & 45 deletions .github/workflows/pr-review-companion.yml
View file
Original file line number Diff line number Diff line change
@@ -1,61 +1,21 @@
# Things to do and run after a "PR Test" workflow has finished successfully.
# Things to do and run after a "tests" job in "PR Test" workflow has completed successfully.
# Note, as of right now, this workflow does a bunch of things. It might be
# worth considering to break it up so there's a dedicated post-PR
# workflow just to posting PR comments about flaws, for example.

name: PR review companion

on:
workflow_run:
workflows: ["PR Test"]
types:
- completed
on: workflow_call

jobs:
review:
runs-on: ubuntu-latest

if: >
${{ github.repository == 'mdn/content' &&
github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.conclusion == 'success' }}
steps:
- name: "Download artifact"
uses: actions/github-script@v6
uses: actions/download-artifact@v3
with:
script: |
var artifacts = await github.rest.actions.listWorkflowRunArtifacts({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: ${{github.event.workflow_run.id }},
});
var matchArtifacts = artifacts.data.artifacts.filter((artifact) => {
return artifact.name == "build"
});
if (matchArtifacts.length === 0) {
console.warn(
'No artifacts to download probably just means nothing ' +
'was built in the "PR test" workflow. That\'s OK. ' +
'This is actually not a genuine CI error.'
);
throw new Error(
'No matched build artifacts. ' +
'Perhaps nothing built in the "PR test" workflow'
);
}
var matchArtifact = matchArtifacts[0];
var download = await github.rest.actions.downloadArtifact({
owner: context.repo.owner,
repo: context.repo.repo,
artifact_id: matchArtifact.id,
archive_format: 'zip',
});
var fs = require('fs');
fs.writeFileSync('${{github.workspace}}/build.zip', Buffer.from(download.data));
- name: Unzip what was downloaded
run: 7z x build.zip -obuild -bb1
name: build
path: build

- uses: actions/checkout@v4
with:
Expand Down
67 changes: 40 additions & 27 deletions .github/workflows/pr-test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,23 @@
name: PR Test

on:
pull_request:
# The `GITHUB_TOKEN` in workflows triggered by the `pull_request_target` event
# is granted read/write repository access.
# Please pay attention to limit the permissions of each job!
# https://docs.github.com/actions/using-jobs/assigning-permissions-to-jobs
pull_request_target:
branches:
- main

jobs:
tests:
if: github.repository == 'mdn/content'
runs-on: ubuntu-latest
# Set the permissions to `read-all`, preventing the workflow from
# any accidental write access to the repository.
permissions: read-all
outputs:
has_assets: ${{ steps.build-content.outputs.has_assets }}
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
Expand All @@ -25,35 +35,10 @@ jobs:

steps:
- uses: actions/checkout@v4

- name: Setup Node.js environment
uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"
cache: yarn

# This is a "required" workflow so path filtering can not be used:
# https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks
# We have to rely on a custom filtering mechanism to run the checks only if required files are modified.
- uses: dorny/paths-filter@v2
name: See if any file needs checking
id: filter
with:
filters: |
required_files :
- ".nvmrc"
- ".github/workflows/pr-test.yml"
- "files/en-us/**"
- name: Install all yarn packages
if: steps.filter.outputs.required_files == 'true'
run: yarn --frozen-lockfile
env:
# https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
ref: "${{ env.HEAD_SHA }}"

- name: Get changed files
if: steps.filter.outputs.required_files == 'true'
run: |
# Use the GitHub API to get the list of changed files
# documentation: https://docs.github.com/rest/commits/commits#compare-two-commits
Expand All @@ -70,7 +55,22 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Setup Node.js environment
if: ${{ env.GIT_DIFF_CONTENT }}
uses: actions/setup-node@v4
with:
node-version-file: ".nvmrc"
cache: yarn

- name: Install all yarn packages
if: ${{ env.GIT_DIFF_CONTENT }}
run: yarn --frozen-lockfile
env:
# https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Build changed content
id: build-content
if: ${{ env.GIT_DIFF_CONTENT }}
env:
CONTENT_ROOT: ${{ github.workspace }}/files
Expand Down Expand Up @@ -119,6 +119,9 @@ jobs:
# be able to use this raw diff file for the benefit of analyzing.
wget https://github.com/${{ github.repository }}/compare/${{ env.BASE_SHA }}...${{ env.HEAD_SHA }}.diff -O ${{ env.BUILD_OUT_ROOT }}/DIFF
# Set the output variable so the next job could skip if there are no assets
echo "has_assets=true" >> "$GITHUB_OUTPUT"
- name: Merge static assets with built documents
if: ${{ env.GIT_DIFF_CONTENT }}
run: |
Expand All @@ -140,3 +143,13 @@ jobs:
export CONTENT_ROOT=$(pwd)/files
yarn filecheck ${{ env.GIT_DIFF_FILES }}
review:
needs: tests
if: ${{ needs.tests.outputs.has_assets }}
# write permissions are required to create a comment in the corresponding PR
permissions: write-all
uses: ./.github/workflows/pr-review-companion.yml
# inherit the secrets from the parent workflow
# https://docs.github.com/actions/using-workflows/reusing-workflows#using-inputs-and-secrets-in-a-reusable-workflow
secrets: inherit

0 comments on commit 9b1c805

Please sign in to comment.