Apply feedback from API review #270
Workflow file for this run
This file contains hidden or 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 workflow finds changed `*.baseline.json` files in a merged PR, | |
| # labels the PR for API review, generates ApiChief diffs, and posts them as PR comments. | |
| name: Comment API baseline deltas on PRs | |
| on: | |
| pull_request_target: | |
| types: [closed] | |
| branches: | |
| - main | |
| - release/** | |
| workflow_dispatch: | |
| inputs: | |
| pr-number: | |
| description: Pull request number to process | |
| required: true | |
| type: number | |
| permissions: | |
| contents: read | |
| issues: write | |
| pull-requests: write | |
| jobs: | |
| api-review: | |
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.merged == true | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Detect changed baseline files and add label | |
| id: detect | |
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 | |
| with: | |
| script: | | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const prNumberInput = context.eventName === 'workflow_dispatch' | |
| ? context.payload.inputs?.['pr-number'] | |
| : context.payload.pull_request.number; | |
| const prNumber = Number(prNumberInput); | |
| if (!Number.isInteger(prNumber) || prNumber <= 0) { | |
| core.setFailed(`Invalid PR number: ${prNumber}`); | |
| return; | |
| } | |
| const { data: pullRequest } = await github.rest.pulls.get({ | |
| owner, | |
| repo, | |
| pull_number: prNumber | |
| }); | |
| let baseSha = pullRequest.base.sha; | |
| const targetSha = pullRequest.merge_commit_sha ?? pullRequest.head.sha; | |
| // pull_request_target's `base.sha` is the original base from when the PR was opened, so it | |
| // includes anything pushed to the target branch while the PR was open. The merge commit's | |
| // first parent is the target tip immediately before the merge — that gives a diff that | |
| // reflects only what the PR introduced. | |
| if (pullRequest.merged && pullRequest.merge_commit_sha) { | |
| try { | |
| const { data: mergeCommit } = await github.rest.git.getCommit({ | |
| owner, | |
| repo, | |
| commit_sha: pullRequest.merge_commit_sha | |
| }); | |
| if (mergeCommit.parents.length > 0) { | |
| baseSha = mergeCommit.parents[0].sha; | |
| console.log(`Using merge commit's first parent ${baseSha} as base.`); | |
| } | |
| } catch (err) { | |
| console.log(`Failed to fetch merge commit; falling back to PR base.sha: ${err.message}`); | |
| } | |
| } | |
| const files = await github.paginate(github.rest.pulls.listFiles, { | |
| owner, | |
| repo, | |
| pull_number: prNumber, | |
| per_page: 100 | |
| }); | |
| const baselineFiles = files | |
| .filter(file => file.filename.endsWith('.baseline.json')) | |
| .map(file => ({ | |
| filename: file.filename, | |
| status: file.status, | |
| previous_filename: file.previous_filename ?? null | |
| })); | |
| core.setOutput('pr_number', String(prNumber)); | |
| core.setOutput('base_sha', baseSha); | |
| core.setOutput('target_sha', targetSha); | |
| core.setOutput('has_baselines', String(baselineFiles.length > 0)); | |
| core.setOutput('files_json', JSON.stringify(baselineFiles)); | |
| if (!pullRequest.merged) { | |
| console.log(`PR #${prNumber} is not merged; using head SHA ${pullRequest.head.sha}.`); | |
| } | |
| if (baselineFiles.length === 0) { | |
| console.log(`No changed baseline files detected on PR #${prNumber}.`); | |
| return; | |
| } | |
| await github.rest.issues.addLabels({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| labels: ['api-review'] | |
| }); | |
| console.log(`Detected ${baselineFiles.length} changed baseline file(s) on PR #${prNumber}.`); | |
| - name: Check out selected commit | |
| if: steps.detect.outputs.has_baselines == 'true' | |
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 | |
| with: | |
| ref: ${{ steps.detect.outputs.target_sha }} | |
| fetch-depth: 1 | |
| - name: Restore repo-local .NET SDK | |
| if: steps.detect.outputs.has_baselines == 'true' | |
| shell: bash | |
| run: ./restore.sh | |
| - name: Build ApiChief and compute review diffs | |
| if: steps.detect.outputs.has_baselines == 'true' | |
| id: delta | |
| shell: bash | |
| env: | |
| FILES_JSON: ${{ steps.detect.outputs.files_json }} | |
| OWNER: ${{ github.repository_owner }} | |
| REPO: ${{ github.event.repository.name }} | |
| BASE_SHA: ${{ steps.detect.outputs.base_sha }} | |
| TARGET_SHA: ${{ steps.detect.outputs.target_sha }} | |
| run: | | |
| set -euo pipefail | |
| DOTNET="$PWD/.dotnet/dotnet" | |
| APICHIEF_PROJECT="$PWD/eng/Tools/ApiChief/ApiChief.csproj" | |
| WORKDIR="$PWD/artifacts/tmp/api-review" | |
| mkdir -p "$WORKDIR" | |
| "$DOTNET" build "$APICHIEF_PROJECT" --configuration Release --nologo --verbosity minimal | |
| APICHIEF_PATH="$($DOTNET msbuild "$APICHIEF_PROJECT" --getProperty:TargetPath -p:Configuration=Release --nologo | tr -d '\r')" | |
| export DOTNET APICHIEF_PATH WORKDIR | |
| python3 - <<'PY' | |
| import json | |
| import os | |
| import pathlib | |
| import subprocess | |
| import urllib.error | |
| import urllib.parse | |
| import urllib.request | |
| files = json.loads(os.environ['FILES_JSON']) | |
| owner = os.environ['OWNER'] | |
| repo = os.environ['REPO'] | |
| base_sha = os.environ['BASE_SHA'] | |
| target_sha = os.environ['TARGET_SHA'] | |
| dotnet = os.environ['DOTNET'] | |
| apichief = os.environ['APICHIEF_PATH'] | |
| workdir = pathlib.Path(os.environ['WORKDIR']) | |
| workdir.mkdir(parents=True, exist_ok=True) | |
| def download_file(ref: str, path: str, destination: pathlib.Path) -> bool: | |
| encoded_path = urllib.parse.quote(path, safe='/') | |
| url = f'https://raw.githubusercontent.com/{owner}/{repo}/{ref}/{encoded_path}' | |
| request = urllib.request.Request(url, headers={'User-Agent': 'efcore-api-review-workflow'}) | |
| try: | |
| with urllib.request.urlopen(request) as response: | |
| destination.write_bytes(response.read()) | |
| return True | |
| except urllib.error.HTTPError as error: | |
| if error.code == 404: | |
| return False | |
| raise | |
| max_comment_length = 60000 | |
| comment_bodies: list[str] = [] | |
| current_body = '' | |
| for index, file in enumerate(files): | |
| filename = file['filename'] | |
| old_path = workdir / f'{index}.old.baseline.json' | |
| new_path = workdir / f'{index}.new.baseline.json' | |
| review_dir = workdir / f'{index}.review' | |
| old_exists = download_file(base_sha, filename, old_path) | |
| new_exists = download_file(target_sha, filename, new_path) | |
| section = '' | |
| if old_exists and new_exists: | |
| result = subprocess.run( | |
| [dotnet, apichief, str(new_path), 'emit', 'delta', str(old_path), '--diff', '-o', str(review_dir)], | |
| capture_output=True, | |
| text=True, | |
| check=False) | |
| if result.returncode == 2: | |
| continue | |
| if result.returncode != 0: | |
| raise RuntimeError( | |
| f'ApiChief delta diff failed for {filename} with exit code {result.returncode}:\n' | |
| f'{result.stdout}\n{result.stderr}') | |
| review_files = sorted(review_dir.rglob('*.md')) | |
| if not review_files: | |
| continue | |
| section = ( | |
| f"## API review baseline changes for `{filename}`\n\n" | |
| "<details>\n<summary><strong>Show diff</strong></summary>\n\n" | |
| 'The diff below was generated by `ApiChief` between the base and the PR.\n\n') | |
| for review_file in review_files: | |
| section += ( | |
| f"{review_file.read_text(encoding='utf-8').rstrip()}\n\n") | |
| section = section.rstrip() + '\n\n</details>' | |
| elif new_exists: | |
| section = ( | |
| f"## API review baseline changes for `{filename}`\n\n" | |
| 'This baseline file was **added** in the PR.') | |
| elif old_exists: | |
| section = ( | |
| f"## API review baseline changes for `{filename}`\n\n" | |
| 'This baseline file was **removed** in the PR.') | |
| if not section: | |
| continue | |
| if len(section) > max_comment_length: | |
| section = ( | |
| f"## API review baseline changes for `{filename}`\n\n" | |
| ':warning: The generated diff for this baseline file exceeds the GitHub comment size limit ' | |
| 'and cannot be displayed. Review the baseline changes locally.') | |
| if not current_body: | |
| current_body = section | |
| elif len(current_body) + 2 + len(section) > max_comment_length: | |
| comment_bodies.append(current_body) | |
| current_body = section | |
| else: | |
| current_body += '\n\n' + section | |
| if not current_body: | |
| current_body = ( | |
| '## API review baseline changes\n\n' | |
| 'No API deltas were produced for the modified baseline files.') | |
| if current_body: | |
| comment_bodies.append(current_body) | |
| comments_dir = workdir / 'comments' | |
| comments_dir.mkdir(parents=True, exist_ok=True) | |
| for index, body in enumerate(comment_bodies, start=1): | |
| comment_path = comments_dir / f'{index:03}.md' | |
| comment_path.write_text(body.rstrip() + '\n', encoding='utf-8') | |
| with open(os.environ['GITHUB_OUTPUT'], 'a', encoding='utf-8') as output: | |
| output.write(f'comments_dir={comments_dir}\n') | |
| PY | |
| - name: Create PR comment with diffs | |
| if: steps.detect.outputs.has_baselines == 'true' | |
| uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 | |
| env: | |
| COMMENTS_DIR: ${{ steps.delta.outputs.comments_dir }} | |
| PR_NUMBER: ${{ steps.detect.outputs.pr_number }} | |
| with: | |
| script: | | |
| const fs = require('fs'); | |
| const path = require('path'); | |
| const owner = context.repo.owner; | |
| const repo = context.repo.repo; | |
| const issue_number = Number(process.env.PR_NUMBER); | |
| const commentsDir = process.env.COMMENTS_DIR; | |
| const commentFiles = fs.readdirSync(commentsDir) | |
| .filter(file => file.endsWith('.md')) | |
| .sort((a, b) => a.localeCompare(b)); | |
| for (const file of commentFiles) { | |
| const body = fs.readFileSync(path.join(commentsDir, file), 'utf8'); | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number, | |
| body | |
| }); | |
| } | |
| console.log(`Created ${commentFiles.length} API review comment(s) for PR #${issue_number}.`); |