Code review feedback.

Author: tjprescott

.github/package.json

@@ -0,0 +1,4 @@
+{
+  "private": true,
+  "type": "module"
+}

.github/shared/src/exec.js

@@ -7,6 +7,7 @@ const execFileImpl = promisify(child_process.execFile);
  * @typedef {Object} ExecOptions
  * @property {string} [cwd] Current working directory.  Default: process.cwd().
  * @property {import('./logger.js').ILogger} [logger]
+ * @property {boolean} [logOutput] Log captured stdout/stderr at info level. Default: false.
  * @property {number} [maxBuffer] Max bytes allowed on stdout or stderr.  Default: 16 * 1024 * 1024.
  */
 
@@ -54,6 +55,7 @@ export async function execFile(file, args, options = {}) {
   const {
     cwd,
     logger,
+    logOutput = false,
     // Node default is 1024 * 1024, which is too small for some git commands returning many entities or large file content.
     // To support "git show", should be larger than the largest swagger file in the repo (2.5 MB as of 2/28/2025).
     maxBuffer = 16 * 1024 * 1024,
@@ -70,11 +72,27 @@ export async function execFile(file, args, options = {}) {
 
     logger?.debug(`stdout: '${result.stdout}'`);
     logger?.debug(`stderr: '${result.stderr}'`);
+    if (logOutput) {
+      if (result.stdout) {
+        logger?.info(result.stdout.trimEnd());
+      }
+      if (result.stderr) {
+        logger?.info(result.stderr.trimEnd());
+      }
+    }
 
     return result;
   } catch (error) {
     /* v8 ignore next */
     logger?.debug(`error: '${JSON.stringify(error)}'`);
+    if (logOutput && isExecError(error)) {
+      if (error.stdout) {
+        logger?.info(error.stdout.trimEnd());
+      }
+      if (error.stderr) {
+        logger?.info(error.stderr.trimEnd());
+      }
+    }
 
     throw error;
   }

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

@@ -1,13 +1,16 @@
 #!/usr/bin/env node
 
-const fs = require("fs");
-const path = require("path");
+import fs from "fs";
+import path from "path";
+import { fileURLToPath, pathToFileURL } from "url";
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
 const DEFAULT_CONFIG = {
   adapter: "python",
 };
 
-function loadWorkflowConfig() {
+export function loadWorkflowConfig() {
   const configPath = path.join(__dirname, "api_md_workflow.config.json");
   if (!fs.existsSync(configPath)) {
     return { ...DEFAULT_CONFIG };
@@ -33,17 +36,11 @@ function loadWorkflowConfig() {
   };
 }
 
-function loadAdapter(name) {
+export async function loadAdapter(name) {
   const adapterPath = path.join(__dirname, "adapters", `${name}.js`);
   if (!fs.existsSync(adapterPath)) {
     throw new Error(`ERROR: adapter '${name}' not found at ${adapterPath}`);
   }
 
-  // eslint-disable-next-line global-require, import/no-dynamic-require
-  return require(adapterPath);
+  return import(pathToFileURL(adapterPath).href);
 }
-
-module.exports = {
-  loadWorkflowConfig,
-  loadAdapter,
-};

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

@@ -1,8 +1,8 @@
 #!/usr/bin/env node
 
-const fs = require("fs");
-const path = require("path");
-const { spawnSync } = require("child_process");
+import fs from "fs";
+import path from "path";
+import { spawnSync } from "child_process";
 
 function run(cmd, args, options = {}) {
   const logger = options.logger || console;
@@ -152,9 +152,10 @@ function generateApiForPackage({
 // Fields in api.metadata.yml that must match between working tree and committed version.
 // pythonVersion is excluded because it varies across CI environments.
 const metadataFieldsToValidate = ["apiMdSha256", "parserVersion"];
+const name = "python";
 
-module.exports = {
-  name: "python",
+export {
+  name,
   isPackageDir,
   findPackageDir,
   readVersion,

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

@@ -1,9 +1,7 @@
-const fs = require("fs");
-const path = require("path");
-const { pathToFileURL } = require("url");
+import fs from "fs";
+import path from "path";
+import { execFile } from "../../../shared/src/exec.js";
 
-const SHARED_SRC_ROOT = path.resolve(__dirname, "..", "..", "..", "shared", "src");
-const sharedModuleCache = new Map();
 const ANSI = {
   bold: "\u001b[1m",
   cyan: "\u001b[36m",
@@ -15,45 +13,12 @@ function styleLog(text, ...styles) {
   return `${styles.join("")}${text}${ANSI.reset}`;
 }
 
-async function loadSharedModule(fileName) {
-  if (sharedModuleCache.has(fileName)) {
-    return sharedModuleCache.get(fileName);
-  }
-
-  const filePath = path.join(SHARED_SRC_ROOT, fileName);
-  const modulePromise = import(pathToFileURL(filePath).href);
-  sharedModuleCache.set(fileName, modulePromise);
-  return modulePromise;
-}
-
 async function runNode(scriptRelativePath, workspace, core) {
-  const { execFile, isExecError } = await loadSharedModule("exec.js");
-
-  try {
-    const result = await execFile("node", [scriptRelativePath], {
-      cwd: workspace,
-      logger: core,
-    });
-
-    if (result.stdout) {
-      core.info(result.stdout.trimEnd());
-    }
-    if (result.stderr) {
-      core.info(result.stderr.trimEnd());
-    }
-  } catch (error) {
-    if (isExecError(error)) {
-      if (error.stdout) {
-        core.info(error.stdout.trimEnd());
-      }
-      if (error.stderr) {
-        core.info(error.stderr.trimEnd());
-      }
-    }
-
-    const status = isExecError(error) && Number.isInteger(error.code) ? error.code : 1;
-    throw new Error(`Command failed (${status}): node ${scriptRelativePath}`);
-  }
+  await execFile("node", [scriptRelativePath], {
+    cwd: workspace,
+    logger: core,
+    logOutput: true,
+  });
 }
 
 function readLines(fileRelativePath, workspace) {
@@ -90,7 +55,7 @@ function formatIssueSection(title, apiFiles) {
   return lines.join("\n");
 }
 
-module.exports = async function apiMdConsistency({ core }) {
+export default async function apiMdConsistency({ core }) {
   const workspace = process.env.GITHUB_WORKSPACE || process.cwd();
 
   await runNode(".github/workflows/src/api-md-consistency/find_affected.js", workspace, core);

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

@@ -1,31 +1,19 @@
 #!/usr/bin/env node
 
-const fs = require("fs");
-const path = require("path");
-const { pathToFileURL } = require("url");
+import fs from "fs";
+import path from "path";
+import { fileURLToPath } from "url";
+import { execFile, isExecError } from "../../../shared/src/exec.js";
+import { defaultLogger } from "../../../shared/src/logger.js";
 
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
 const REPO_ROOT = path.resolve(__dirname, "..", "..", "..", "..");
-const SHARED_SRC_ROOT = path.join(REPO_ROOT, ".github", "shared", "src");
-const sharedModuleCache = new Map();
-
-async function loadSharedModule(fileName) {
-  if (sharedModuleCache.has(fileName)) {
-    return sharedModuleCache.get(fileName);
-  }
-
-  const filePath = path.join(SHARED_SRC_ROOT, fileName);
-  const modulePromise = import(pathToFileURL(filePath).href);
-  sharedModuleCache.set(fileName, modulePromise);
-  return modulePromise;
-}
 
 async function getDefaultLogger() {
-  const { defaultLogger } = await loadSharedModule("logger.js");
   return defaultLogger;
 }
 
 async function runAsync(cmd, args, options = {}) {
-  const { execFile, isExecError } = await loadSharedModule("exec.js");
   const check = options.check ?? true;
   const logger = options.logger ?? (await getDefaultLogger());
 
@@ -100,9 +88,8 @@ function requireEnv(name) {
   return value;
 }
 
-module.exports = {
+export {
   REPO_ROOT,
-  loadSharedModule,
   getDefaultLogger,
   runAsync,
   readLines,

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

@@ -1,23 +1,21 @@
 #!/usr/bin/env node
 
-const {
+import {
   REPO_ROOT,
   appendGithubOutput,
   envPath,
   getDefaultLogger,
-  loadSharedModule,
   requireEnv,
   runAsync,
   writeLines,
-} = require("./common");
-const { loadAdapter, loadWorkflowConfig } = require("./adapter_config");
+} from "./common.js";
+import { loadAdapter, loadWorkflowConfig } from "./adapter_config.js";
+import { includesSegment } from "../../../shared/src/path.js";
 
 async function main() {
-  const { includesSegment } = await loadSharedModule("path.js");
-
   const config = loadWorkflowConfig();
   const adapterName = config.adapter;
-  const adapter = loadAdapter(adapterName);
+  const adapter = await loadAdapter(adapterName);
   if (typeof adapter.isPackageDir !== "function") {
     throw new Error(`ERROR: adapter '${adapterName}' does not implement isPackageDir(repoRoot, packageDirRelative).`);
   }

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

@@ -1,9 +1,9 @@
 #!/usr/bin/env node
 
-const fs = require("fs");
+import fs from "fs";
 
-const { appendGithubOutput, envPath, getDefaultLogger, readLines, runAsync, writeLines } = require("./common");
-const { loadAdapter, loadWorkflowConfig } = require("./adapter_config");
+import { appendGithubOutput, envPath, getDefaultLogger, readLines, runAsync, writeLines } from "./common.js";
+import { loadAdapter, loadWorkflowConfig } from "./adapter_config.js";
 
 /**
  * Parse a simple key: value YAML file into an object.
@@ -22,7 +22,7 @@ function parseSimpleYaml(text) {
 
 async function main() {
   const config = loadWorkflowConfig();
-  const adapter = loadAdapter(config.adapter);
+  const adapter = await loadAdapter(config.adapter);
 
   // Fields to compare in api.metadata.yml. If the adapter doesn't specify,
   // compare all fields (strict default for languages that don't opt out).

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

@@ -1,14 +1,14 @@
 #!/usr/bin/env node
 
-const path = require("path");
+import path from "path";
 
-const { REPO_ROOT, envPath, getDefaultLogger, readLines } = require("./common");
-const { loadAdapter, loadWorkflowConfig } = require("./adapter_config");
+import { REPO_ROOT, envPath, getDefaultLogger, readLines } from "./common.js";
+import { loadAdapter, loadWorkflowConfig } from "./adapter_config.js";
 
 async function main() {
   const logger = await getDefaultLogger();
   const config = loadWorkflowConfig();
-  const adapter = loadAdapter(config.adapter);
+  const adapter = await loadAdapter(config.adapter);
   if (typeof adapter.generateApiForPackage !== "function") {
     throw new Error(
       `ERROR: adapter '${config.adapter}' does not implement generateApiForPackage({ repoRoot, packageName, runtimeExecutable }).`,

eng/tools/azure-sdk-tools/azpysdk/apistub.py

@@ -81,7 +81,10 @@ def register(
             dest="install_deps",
             default=False,
             action="store_true",
-            help="Install dev requirements and apiview dependencies before running. Skipped by default for faster local iteration.",
+            help=(
+                "Install dev requirements and apiview dependencies before running. "
+                "Skipped by default for faster local iteration, but always enabled when --isolate is used."
+            ),
         )
         p.set_defaults(func=self.run)