-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
refactor(ci/pr-review): use workflow_call
instead of workflow_run
event
#16891
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7b3606a
refactor(ci/pr-review): use `workflow_call` instead of `workflow_run`…
yin1999 67988cd
chore: remove path matches
yin1999 8065dc7
Merge branch 'main' into pr-review
yin1999 d9b1071
setup the environment when the non-markdown files are changed
yin1999 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
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 |
---|---|---|
|
@@ -7,19 +7,24 @@ | |
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 | ||
paths: | ||
- .nvmrc | ||
- ".github/workflows/pr-test.yml" | ||
- "files/**" | ||
|
||
jobs: | ||
tests: | ||
# do not run on PRs in forks | ||
if: github.repository == 'mdn/translated-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 }} | ||
|
@@ -30,35 +35,13 @@ jobs: | |
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
# Needed otherswise it will check out a merge commit which means | ||
# you can't get a `git diff` for this PR compared to its base. | ||
# with: | ||
# ref: ${{ github.event.pull_request.head.sha }} | ||
|
||
- uses: actions/checkout@v4 | ||
with: | ||
repository: mdn/content | ||
path: mdn/content | ||
|
||
- name: Setup Node.js environment | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version-file: ".nvmrc" | ||
cache: "yarn" | ||
cache-dependency-path: mdn/content/yarn.lock | ||
|
||
- name: Install all yarn packages | ||
working-directory: ${{ github.workspace }}/mdn/content | ||
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 | ||
run: | | ||
# Use the GitHub API to get the list of changed files | ||
# documenation: https://docs.github.com/rest/commits/commits#compare-two-commits | ||
# documentation: https://docs.github.com/rest/commits/commits#compare-two-commits | ||
DIFF_DOCUMENTS=$(gh api repos/{owner}/{repo}/compare/${{ env.BASE_SHA }}...${{ env.HEAD_SHA }} \ | ||
--jq '.files | .[] | select(.status|IN("added", "modified", "renamed", "copied", "changed")) | .filename') | ||
|
||
|
@@ -73,15 +56,38 @@ jobs: | |
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- uses: actions/checkout@v4 | ||
if: ${{ env.GIT_DIFF_CONTENT }} || ${{ env.GIT_DIFF_FILES }} | ||
with: | ||
repository: mdn/content | ||
path: mdn/content | ||
|
||
- name: Setup Node.js environment | ||
if: ${{ env.GIT_DIFF_CONTENT }} || ${{ env.GIT_DIFF_FILES }} | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version-file: ".nvmrc" | ||
cache: yarn | ||
cache-dependency-path: mdn/content/yarn.lock | ||
|
||
- name: Install all yarn packages | ||
if: ${{ env.GIT_DIFF_CONTENT }} || ${{ env.GIT_DIFF_FILES }} | ||
working-directory: ${{ github.workspace }}/mdn/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 }}/mdn/content/files | ||
CONTENT_TRANSLATED_ROOT: ${{ github.workspace }}/files | ||
|
||
# This is so that if there's a single 'unsafe_html' flaw, it | ||
# completely fails the build. | ||
# But all other flaws should be 'warn' so that we can include | ||
# But all other flaws should be 'warn', so that we can include | ||
# information about the flaws when we analyze the built PR. | ||
BUILD_FLAW_LEVELS: "unsafe_html: error, *:warn" | ||
|
||
|
@@ -94,7 +100,7 @@ jobs: | |
BUILD_LIVE_SAMPLES_BASE_URL: https://live.mdnyalp.dev | ||
BUILD_LEGACY_LIVE_SAMPLES_BASE_URL: https://live-samples.mdn.allizom.net | ||
|
||
# In these builds we never care for or need the ability to sign in. | ||
# In these builds, we never care for or need the ability to sign in. | ||
# This environment variable will disable that functionality entirely. | ||
REACT_APP_DISABLE_AUTH: true | ||
|
||
|
@@ -112,21 +118,21 @@ jobs: | |
# the BUILD_OUT_ROOT and CONTENT_ROOT env vars. | ||
node node_modules/@mdn/yari/build/cli.js ${{ env.GIT_DIFF_CONTENT }} | ||
|
||
echo "Disk usage size of build/" | ||
echo "Disk usage size of build" | ||
du -sh ${{ env.BUILD_OUT_ROOT }} | ||
|
||
# Save the PR number into the build | ||
echo ${{ github.event.number }} > ${{ env.BUILD_OUT_ROOT }}/NR | ||
|
||
- name: Download the diff file | ||
if: ${{ env.GIT_DIFF_CONTENT }} | ||
run: | | ||
# Save the raw diff blob and store that inside the ./build/ | ||
# Download the raw diff blob and store that inside the build | ||
# directory. | ||
# The purpose of this is for the PR Review Companion to later | ||
# 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 | ||
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. Merge the steps, align with the mdn/content's workflow |
||
|
||
# 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: | | ||
|
@@ -151,3 +157,13 @@ jobs: | |
echo ${{ env.GIT_DIFF_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 |
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.
Slight differences here: https://www.diffchecker.com/bU2WdBmN/, you may want to grab the latest version from
main
as there were some bugfixes on "non-markdown only" changed files, e.g.:if: ${{ env.GIT_DIFF_CONTENT }} || ${{ env.GIT_DIFF_FILES }
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.
Which reminds me that these workflows should probably (eventually) live in https://github.com/mdn/workflows as they will diverge over time.
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.
fixed in d9b1071