Python POC for GitHub-based API Reviews (Phase 2)#47443
Open
tjprescott wants to merge 55 commits into
Open
Conversation
Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com>
Co-authored-by: tjprescott <5723682+tjprescott@users.noreply.github.com>
…e-sdk-for-python into GenerateAPITextScript
tjprescott
changed the title
…into APIReviewSync # Conflicts: # .github/workflows/api-consistency.yml # .github/workflows/api-md-workflow-tests.yml # eng/tools/azure-sdk-tools/azpysdk/apistub.py # eng/tools/azure-sdk-tools/tests/test_apistub.py # scripts/api_md_workflow/create_api_review_pr.py # scripts/api_md_workflow/create_api_review_pr_test.py
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements “Phase 2” of the GitHub-based API review prototype by adding a dispatcher + sync mechanism that keeps open API review PR branches in sync with validated api.md / api.metadata.yml artifacts from a working branch once CI gates pass.
Changes:
- Adds a dispatcher script and workflow to evaluate a “final CI gate”, discover matching open
[API Review]PRs via embedded metadata, and dispatch per-review-branch sync runs. - Adds a sync script and workflow that compares/copies
api.md+api.metadata.ymlfrom a validated working SHA into the review branch and force-pushes with lease when changes exist. - Consolidates Node tooling dependencies under
.github/package.json(removing the separate.github/chronuspackage definition/lock).
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/api_md_workflow/sync_review_branches_plan.md | Design/implementation plan describing gating, discovery, sync behavior, and safety checks. |
| .github/workflows/src/api-md-review-sync/sync_review_branch.test.js | Unit tests for artifact diff/copy behavior and stale-SHA skipping. |
| .github/workflows/src/api-md-review-sync/sync_review_branch.js | Sync implementation for comparing/copying API artifacts and pushing updates to review branches. |
| .github/workflows/src/api-md-review-sync/dispatch_review_branch_syncs.test.js | Unit tests for metadata parsing, PR matching, dispatching, and CI gate evaluation. |
| .github/workflows/src/api-md-review-sync/dispatch_review_branch_syncs.js | Dispatcher implementation: final CI gating, metadata parsing, PR discovery, and workflow dispatch. |
| .github/workflows/api-md-sync-review-branch.yml | Workflow to perform a single review-branch sync from working SHA artifacts. |
| .github/workflows/api-md-dispatch-review-syncs.yml | Workflow to resolve final CI gate and dispatch sync workflows for matching review PRs. |
| .github/shared/src/simple-git.js | Shared helper to resolve a git repo root via simple-git. |
| .github/shared/src/github.js | Shared GitHub-related enum constants/types (status/check terminology). |
| .github/shared/src/changed-files.js | Shared helper to compute changed files via git diff (now enables simple-git debug). |
| .github/package.json | Consolidated/pinned Node dependencies for GitHub automation and workflows. |
| .github/chronus/package.json | Removed (Chronus tooling dependency definition moved to .github/package.json). |
| .github/chronus/package-lock.json | Removed (Chronus lockfile no longer needed under .github/chronus). |
Files not reviewed (1)
- .github/chronus/package-lock.json: Generated file
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh run download "${{ steps.gate.outputs.consistency_run_id }}" --name api-md-affected-packages --dir .artifacts |
Comment on lines
+280
to
+286
| const pullRequest = selectWorkingPullRequest(await getCommitPullRequestsFn(workingSha), workingSha); | ||
| if (!pullRequest) { | ||
| return { ready: false, reason: `No open pull request found for ${workingSha}.` }; | ||
| } | ||
| if (pullRequest.head.ref.startsWith("apireview/")) { | ||
| return { ready: false, reason: `Skipping API review branch ${pullRequest.head.repo.owner.login}:${pullRequest.head.ref}.` }; | ||
| } |
Comment on lines
+256
to
+258
| const statusPassed = status && PASSING_STATUS_STATES.has(status.state); | ||
| const checkRunPassed = | ||
| checkRun && !isCurrentDispatcherRun(checkRun, currentRunId) && checkRun.status === "completed" && PASSING_CHECK_CONCLUSIONS.has(checkRun.conclusion); |
Comment on lines
+40
to
+43
| return workingPaths.some((workingPath, index) => { | ||
| const reviewPath = reviewPaths[index]; | ||
| return !readRequiredFile(workingPath).equals(Buffer.from(readFileOrUndefined(reviewPath) || "")); | ||
| }); |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Outlines the process for ensuring that changes to working branches are synced to any open review PRs.
Adds implementation of the proposed pipelines.