Skip to content

Commit 818a5ee

Browse files
committed
Make review PR creation only work for updates, not new packages.
1 parent 317bd4c commit 818a5ee

7 files changed

Lines changed: 92 additions & 87 deletions

File tree

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
---
22
name: create-api-review-pr
3-
description: Create a GitHub PR for API review by comparing a baseline API surface against a target branch. Use this when the user wants to create an API review PR, compare API changes between versions, or review API surface differences for a package.
3+
description: Create a GitHub PR for API review by comparing a baseline API surface against a target tag or branch. Use this when the user wants to create an API review PR, compare API changes between versions, or review API surface differences for a package.
44
---
55

66
# Create API Review PR
77

8-
Creates a dedicated API review PR that shows the diff between a baseline release and a target branch's API surface using `scripts/api_md_workflow/create_api_review_pr.js`.
8+
Creates a dedicated API review PR that shows the diff between a baseline release and a target tag or branch's API surface using `scripts/api_md_workflow/create_api_review_pr.js`.
9+
10+
## Unsupported Requests
11+
12+
If the user asks to create an API review PR for a new package, explain that new packages do not use API review PRs and stop. Do not gather script inputs or run `create_api_review_pr.js` for new packages.
913

1014
## Prerequisites
1115

@@ -21,14 +25,14 @@ Ask the user for the following using `vscode_askQuestions`:
2125
### 1. Package Name (required)
2226
The Azure SDK package name (e.g. `azure-storage-blob`, `azure-ai-projects`).
2327

24-
### 2. Baseline (optional)
28+
### 2. Baseline (required)
2529
The release tag to use as the baseline for comparison. Tags follow the format `<package-name>_<version>` (e.g. `azure-storage-blob_12.29.0`).
2630

2731
- If the user provides a package name and version separately, construct the tag as `<package-name>_<version>`.
28-
- If this is a **new package** with no prior release, the baseline should be omitted (the script handles this as an empty baseline).
2932

3033
### 3. Target (optional)
3134
The branch or PR to generate the "current" API surface from. Can be:
35+
- A package release tag (e.g. `azure-storage-blob_12.30.0`) — used directly as a tag ref
3236
- A branch name (e.g. `main`, `feature-branch`) — fetched from `origin`
3337
- An `owner:branch` reference (e.g. `someone:their-branch`) — fetched from the fork
3438
- If omitted, defaults to `origin/main`
@@ -38,15 +42,20 @@ The branch or PR to generate the "current" API surface from. Can be:
3842
Before running the script:
3943

4044
1. **Validate the package exists**: Confirm a directory matching `sdk/*/<package-name>` exists with a `pyproject.toml` or `setup.py`.
41-
2. **Validate the baseline tag** (if provided): Run `git tag -l "<tag>"` to confirm the tag exists. If the user provided a version like `12.29.0`, construct the full tag as `<package-name>_<version>` and validate that.
42-
3. **Confirm the working tree is clean**: Run `git status --porcelain` and warn if there are uncommitted changes.
45+
2. **Validate the baseline tag**: Run `git tag -l "<tag>"` to confirm the tag exists. If the user provided a version like `12.29.0`, construct the full tag as `<package-name>_<version>` and validate that.
46+
3. **Validate the target tag when applicable**: If the user provided a target version or tag, construct or validate the full tag as `<package-name>_<version>` and run `git tag -l "<tag>"`.
47+
4. **Confirm the working tree is clean**: Run `git status --porcelain` and warn if there are uncommitted changes.
4348

4449
## Execution
4550

51+
This is a long-running operation. The script may take several minutes because it generates API surfaces for both the baseline and target, installs package dependencies, creates or reuses review branches, pushes branches, and then opens the draft PR. Do not treat quiet terminal periods during dependency installation or `apistub` generation as failure unless the command exits, prints an error, or waits for input.
52+
53+
If `create_api_review_pr.js` fails while running this skill, do not patch the script, modify package files, retry with workaround edits, or try to manually complete branch/PR creation. Stop the workflow, report the failure clearly, include the relevant error details, and suggest practical next steps.
54+
4655
Run the following command from the repository root:
4756

4857
```bash
49-
node scripts/api_md_workflow/create_api_review_pr.js --package-name <package-name> [--base <tag>] [--target <target>]
58+
node scripts/api_md_workflow/create_api_review_pr.js --package-name <package-name> --base <tag> [--target <target>]
5059
```
5160

5261
### Examples
@@ -56,21 +65,25 @@ node scripts/api_md_workflow/create_api_review_pr.js --package-name <package-nam
5665
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-storage-blob --base azure-storage-blob_12.29.0 --target someone:feature-branch
5766
```
5867

59-
**Review against main (no target specified):**
68+
**Release-to-release review (comparing two package tags):**
6069
```bash
61-
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-cosmos --base azure-cosmos_4.14.0
70+
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-ai-projects --base azure-ai-projects_2.1.0 --target azure-ai-projects_2.2.0
6271
```
6372

64-
**New package (no baseline):**
73+
**Review against main (no target specified):**
6574
```bash
66-
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-keyvault-secrets --target main
75+
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-cosmos --base azure-cosmos_4.14.0
6776
```
6877

6978
## Post-Execution
7079

7180
The script will:
7281
1. Generate `API.md` for both baseline and target
73-
2. Push `base_<package>_<version>` and `review_<package>_<version>` branches
82+
2. Push `apireview/base_<package>_<version>` and `apireview/review_<package>_<version>` branches
7483
3. Open a draft PR (or print a compare URL if `gh pr create` fails)
7584

85+
During execution, report progress at major phases: baseline generation, target generation, branch creation or reuse, branch push, and PR creation. If the terminal is quiet, check whether the process is still running before assuming it is hung.
86+
87+
When the target is a tag, the PR body labels it as `Target tag`. Branch and fork targets are labeled as `Working branch`.
88+
7689
Report the PR URL to the user when complete.

.github/workflows/src/api-md-consistency/api-md-consistency.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function formatIssueSection(title, apiFiles) {
4444
const packageName = path.basename(packageDir);
4545
lines.push(`- ${packageDir}`);
4646
lines.push(` API file: ${apiFile}`);
47-
lines.push(` Regenerate: azpysdk apistub --md --extract-metadata ${packageName} --dest-dir ${packageDir}`);
47+
lines.push(` Regenerate: azpysdk apistub --md --extract-metadata ${packageName} --dest-dir .`);
4848
}
4949
lines.push("");
5050
return lines.join("\n");

scripts/api_md_workflow/README.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ It runs on pull requests for changes under `sdk/**`.
2222
- Regenerates `api.md` for those packages.
2323
- Fails if the generated files differ from the committed files.
2424
- Fails if an affected package does not have a committed `api.md`.
25-
- Prints the mismatched or missing packages and the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir <package-dir>` command needed to regenerate each `api.md` file.
25+
- Prints the mismatched or missing packages and the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir .` command needed to regenerate each `api.md` file from the repository root.
2626

2727
## Script Layout
2828

@@ -49,7 +49,7 @@ Also writes `count=<n>` to `GITHUB_OUTPUT`.
4949

5050
### `regenerate.js`
5151

52-
Reads package directories from `API_MD_PACKAGES_FILE` and runs `azpysdk apistub --md --extract-metadata <package-name> --dest-dir <package-dir>` for each package.
52+
Reads package directories from `API_MD_PACKAGES_FILE` and runs `azpysdk apistub --md --extract-metadata <package-name> --dest-dir <package-dir>` for each package from the repository root.
5353

5454
This script is used by the consistency check.
5555

@@ -73,6 +73,14 @@ API review PR creation now uses a shared JavaScript orchestrator with a language
7373

7474
This split allows the core workflow to be reused across other language repos while keeping generation behavior language-specific.
7575

76+
`create_api_review_pr.js` compares a baseline package release tag with a target API surface. The target can be a package release tag, an `origin` branch, or an `owner:branch` fork reference. When the target is a tag, the generated PR body identifies it as a target tag instead of a working branch.
77+
78+
Example comparing two package release tags:
79+
80+
```bash
81+
node scripts/api_md_workflow/create_api_review_pr.js --package-name azure-ai-projects --base azure-ai-projects_2.1.0 --target azure-ai-projects_2.2.0
82+
```
83+
7684
### `api_md_workflow.config.json`
7785

7886
Shared configuration for adapter selection across `api_md_workflow` scripts.
@@ -100,4 +108,4 @@ Common variables include:
100108
3. `find_affected.js` determines which packages were touched.
101109
4. `regenerate.js` rebuilds `api.md` for those packages.
102110
5. `find_mismatches.js` records any `api.md` drift, including missing or untracked `api.md` files.
103-
6. If drift is found, the workflow fails and prints the affected packages plus the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir <package-dir>` command to regenerate each `api.md` file locally.
111+
6. If drift is found, the workflow fails and prints the affected packages plus the `azpysdk apistub --md --extract-metadata <package-name> --dest-dir .` command to regenerate each `api.md` file locally from the repository root.

scripts/api_md_workflow/create_api_review_pr.js

Lines changed: 54 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ function parseArgs(argv) {
6969
throw new Error("Missing required --package-name");
7070
}
7171

72+
if (!args.base) {
73+
throw new Error("Missing required --base");
74+
}
75+
7276
return args;
7377
}
7478

@@ -179,19 +183,33 @@ function validateBaseTag(packageName, baseTag) {
179183
return version;
180184
}
181185

186+
function resolveTargetTag(target) {
187+
if (tagExists(target)) {
188+
return target;
189+
}
190+
191+
git(["fetch", REMOTE, "tag", target], { check: false, capture: true });
192+
return tagExists(target) ? target : null;
193+
}
194+
182195
function remoteBranchRef(branch) {
183196
git(["fetch", REMOTE, branch]);
184197
return `${REMOTE}/${branch}`;
185198
}
186199

187200
function resolveTargetRef(target) {
201+
const targetTag = resolveTargetTag(target);
202+
if (targetTag) {
203+
return targetTag;
204+
}
205+
188206
if (!target.includes(":")) {
189207
return remoteBranchRef(target);
190208
}
191209

192210
const [owner, branch] = target.split(":", 2);
193211
if (!owner || !branch) {
194-
throw new Error(`ERROR: invalid --target '${target}'. Expected either 'branch' or 'owner:branch'.`);
212+
throw new Error(`ERROR: invalid --target '${target}'. Expected 'tag', 'branch', or 'owner:branch'.`);
195213
}
196214

197215
const forkUrl = `https://github.com/${owner}/azure-sdk-for-python.git`;
@@ -765,6 +783,19 @@ function baselineReferenceMarkdown(baseTag) {
765783
return `[tag \`${baseTag}\`](${commitUrl})`;
766784
}
767785

786+
function targetReferenceMarkdown(headSelector) {
787+
const targetTag = resolveTargetTag(headSelector);
788+
if (targetTag) {
789+
return baselineReferenceMarkdown(targetTag);
790+
}
791+
792+
return workingReferenceMarkdown(headSelector);
793+
}
794+
795+
function targetReferenceLabel(headSelector) {
796+
return resolveTargetTag(headSelector) ? "Target tag" : "Working branch";
797+
}
798+
768799
function writeBytes(filePath, bytes) {
769800
fs.mkdirSync(path.dirname(filePath), { recursive: true });
770801
fs.writeFileSync(filePath, bytes);
@@ -812,6 +843,7 @@ async function generateApiBytesForRef({
812843
return result;
813844
} finally {
814845
// Restore the package directory to the current branch state
846+
git(["reset", "--", packageRelative], { check: false });
815847
git(["checkout", "HEAD", "--", packageRelative]);
816848
// Clean any untracked files that the generation may have left behind
817849
git(["clean", "-fd", "--", packageRelative], { check: false });
@@ -833,28 +865,22 @@ async function main() {
833865

834866
git(["fetch", REMOTE, "main"]);
835867

836-
let baseVersion = "none";
837-
if (args.base) {
838-
baseVersion = validateBaseTag(args.packageName, args.base);
839-
}
868+
const baseVersion = validateBaseTag(args.packageName, args.base);
840869

841870
const targetRef = args.target ? resolveTargetRef(args.target) : MAIN_REF;
842871

843872
try {
844-
let baseResult = null;
845-
if (args.base) {
846-
logInfo(`\n=== Capturing baseline api.md from tag ${args.base} ===`);
847-
baseResult = await generateApiBytesForRef({
848-
adapter,
849-
repoRoot: REPO_ROOT,
850-
packageName: args.packageName,
851-
packageDir,
852-
runtimeExecutable: args.runtimeExecutable,
853-
ref: args.base,
854-
refLabel: args.base,
855-
logger,
856-
});
857-
}
873+
logInfo(`\n=== Capturing baseline api.md from tag ${args.base} ===`);
874+
const baseResult = await generateApiBytesForRef({
875+
adapter,
876+
repoRoot: REPO_ROOT,
877+
packageName: args.packageName,
878+
packageDir,
879+
runtimeExecutable: args.runtimeExecutable,
880+
ref: args.base,
881+
refLabel: args.base,
882+
logger,
883+
});
858884

859885
logInfo(`\n=== Capturing target api.md from ${targetRef} ===`);
860886
const targetResult = await generateApiBytesForRef({
@@ -893,41 +919,13 @@ async function main() {
893919
} else {
894920
logInfo(`\n=== Creating base branch ${baseBranch} ===`);
895921
git(["checkout", "-B", baseBranch, MAIN_REF]);
896-
897-
if (baseResult !== null) {
898-
writeBytes(apiPath, baseResult.apiMd);
899-
git(["add", apiRelative]);
900-
if (baseResult.metadata) {
901-
writeBytes(metaFilePath, baseResult.metadata);
902-
git(["add", metaRelative]);
903-
}
904-
git(["commit", "-m", `[API Review] Baseline api.md for ${args.packageName} ${baseVersion}`]);
905-
} else {
906-
const tracked = git(["ls-files", "--error-unmatch", apiRelative], {
907-
capture: true,
908-
check: false,
909-
});
910-
911-
if (tracked.status === 0) {
912-
git(["rm", apiRelative]);
913-
const metaTracked = git(["ls-files", "--error-unmatch", metaRelative], {
914-
capture: true,
915-
check: false,
916-
});
917-
if (metaTracked.status === 0) {
918-
git(["rm", metaRelative]);
919-
}
920-
git(["commit", "-m", `[API Review] Remove api.md for ${args.packageName} (empty baseline)`]);
921-
} else {
922-
if (fs.existsSync(apiPath)) {
923-
fs.unlinkSync(apiPath);
924-
}
925-
if (fs.existsSync(metaFilePath)) {
926-
fs.unlinkSync(metaFilePath);
927-
}
928-
git(["commit", "--allow-empty", "-m", `[API Review] Empty baseline for ${args.packageName}`]);
929-
}
922+
writeBytes(apiPath, baseResult.apiMd);
923+
git(["add", apiRelative]);
924+
if (baseResult.metadata) {
925+
writeBytes(metaFilePath, baseResult.metadata);
926+
git(["add", metaRelative]);
930927
}
928+
git(["commit", "-m", `[API Review] Baseline api.md for ${args.packageName} ${baseVersion}`]);
931929

932930
git(["push", "--force-with-lease", REMOTE, baseBranch]);
933931
}
@@ -975,13 +973,14 @@ async function main() {
975973

976974
const title = `[API Review] ${args.packageName} ${targetVersion} (base ${baseVersion})`;
977975
const workingSelector = args.target || originalBranch;
978-
const workingRef = workingReferenceMarkdown(workingSelector);
976+
const workingLabel = targetReferenceLabel(workingSelector);
977+
const workingRef = targetReferenceMarkdown(workingSelector);
979978
const baselineRef = baselineReferenceMarkdown(args.base);
980979

981980
const body = [
982981
`Automated API review PR for ${args.packageName}.`,
983982
"",
984-
`- **Working branch:** ${workingRef} (version ${targetVersion})`,
983+
`- **${workingLabel}:** ${workingRef} (version ${targetVersion})`,
985984
`- **Baseline:** ${baselineRef} (version ${baseVersion})`,
986985
"",
987986
"Generated by scripts/api_md_workflow/create_api_review_pr.js.",

sdk/template/azure-template/api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ namespace azure.template.template_code
99
def azure.template.template_code.template_main() -> bool: ...
1010

1111

12-
```
12+
```

sdk/template/azure-template/sdk/template/azure-template/api.md

Lines changed: 0 additions & 12 deletions
This file was deleted.

sdk/template/azure-template/sdk/template/azure-template/api.metadata.yml

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)