Check new py:data pages in metadata checks (#1919)

Follow up to #1917. That resulted in some pages having their behavior fixed, so we can be more strict in our API lints. This PR also makes some bigger improvements to the checks: * Runs `check:metadata` in CI, thanks to a new allowlist * Simplifies `check:orphan-pages` to have an `--apis` arg * DRYs our ignore list for `check:internal-links`
Qiskit · Sep 6, 2024 · 632cb2c · 632cb2c
1 parent 7de29e7
commit 632cb2c
Show file tree

Hide file tree

Showing 5 changed files with 135 additions and 177 deletions.
diff --git a/.github/workflows/api-checks.yml b/.github/workflows/api-checks.yml
@@ -15,11 +15,10 @@ on:
   workflow_dispatch:
   pull_request:
     paths:
-      - "docs/api/qiskit/**/*"
-      - "docs/api/qiskit-ibm-runtime/**/*"
+      - "docs/api/**/*"
       - "public/api/**/*"
-      - "scripts/checkInternalLinks.ts"
-      - "scripts/lib/links/ignores.ts"
+      - "public/images/api/**/*"
+      - "scripts/js/**/*"
 
 jobs:
   api-checks:
@@ -42,8 +41,6 @@ jobs:
           --historical-apis
           --skip-broken-historical
       - name: Check for orphan pages
-        run: >
-          npm run check:orphan-pages --
-          --current-apis
-          --dev-apis
-          --historical-apis
+        run: npm run check:orphan-pages -- --apis
+      - name: Check metadata
+        run: npm run check:metadata -- --apis
diff --git a/README.md b/README.md
@@ -251,18 +251,18 @@ npm run check:external-links -- 'docs/run/*' '!docs/run/index.mdx'
 
 ## Check for orphan pages
 
-Every file should have a home in one of the `_toc.json` files. If for some reason a page should _not_ have a home, add it to the `ALLOWED_ORPHANS` list in `scripts/checkOrphanPages.ts`.
+Every file should have a home in one of the `_toc.json` files.
 
 To check for orphaned pages, run:
 
 ```bash
 # Only check non-API docs
 npm run check:orphan-pages
 
-# You can also add any of the below arguments to check API docs
-npm run check:orphan-pages -- --current-apis --dev-apis --historical-apis
+# You can also check API docs
+npm run check:orphan-pages -- --apis
 
-# Or, run all the checks.  However this will skip the API docs
+# Or, run all the checks. However this will skip the API docs
 npm run check
 ```
 

diff --git a/scripts/js/commands/checkMetadata.ts b/scripts/js/commands/checkMetadata.ts
@@ -17,6 +17,8 @@ import { hideBin } from "yargs/helpers";
 import grayMatter from "gray-matter";
 import { globby } from "globby";
 
+const ALLOWED_VIOLATIONS: Set<string> = new Set([...qiskitLegacyIgnores()]);
+
 interface Arguments {
   [x: string]: unknown;
   apis: boolean;
@@ -29,8 +31,7 @@ const readArgs = (): Arguments => {
     .option("apis", {
       type: "boolean",
       default: false,
-      description:
-        "Check the API docs? Currently fails (https://github.com/Qiskit/documentation/issues/66)",
+      description: "Check the API docs?",
     })
     .option("translations", {
       type: "boolean",
@@ -59,7 +60,7 @@ const isValidMetadata = (
 ): boolean =>
   metadata.title &&
   metadata.description &&
-  (filePath.startsWith("/api/") ||
+  (filePath.startsWith("docs/api/") ||
     (metadata.title != metadata.description &&
       metadata.description.length <= 160 &&
       metadata.description.length >= 50));
@@ -70,6 +71,8 @@ const main = async (): Promise<void> => {
 
   const mdErrors = [];
   for (const file of mdFiles) {
+    if (ALLOWED_VIOLATIONS.has(file)) continue;
+
     const metadata = await readMetadata(file);
     if (!isValidMetadata(metadata, file)) {
       mdErrors.push(file);
@@ -78,6 +81,8 @@ const main = async (): Promise<void> => {
 
   const notebookErrors = [];
   for (const file of notebookFiles) {
+    if (ALLOWED_VIOLATIONS.has(file)) continue;
+
     const metadata = await readMetadata(file);
     if (!isValidMetadata(metadata, file)) {
       notebookErrors.push(file);
@@ -159,4 +164,43 @@ function handleErrors(mdErrors: string[], notebookErrors: string[]): void {
   }
 }
 
-main();
+function qiskitLegacyIgnores(): string[] {
+  const versions = [
+    "0.19/",
+    "0.24/",
+    "0.25/",
+    "0.26/",
+    "0.27/",
+    "0.28/",
+    "0.29/",
+    "0.30/",
+    "0.31/",
+    "0.32/",
+    "0.33/",
+    "0.35/",
+    "0.36/",
+    "0.37/",
+    "0.38/",
+    "0.39/",
+    "0.40/",
+    "0.41/",
+    "0.42/",
+    "0.43/",
+    "0.44/",
+    "0.45/",
+    "0.46/",
+  ];
+  return [
+    ...versions.flatMap((vers) => [
+      `docs/api/qiskit/${vers}aer.mdx`,
+      `docs/api/qiskit/${vers}aqua.mdx`,
+      `docs/api/qiskit/${vers}ibmq-provider.mdx`,
+      `docs/api/qiskit/${vers}ibmq_jupyter.mdx`,
+      `docs/api/qiskit/${vers}ibmq_visualization.mdx`,
+      `docs/api/qiskit/${vers}parallel.mdx`,
+      `docs/api/qiskit/${vers}transpiler_builtin_plugins.mdx`,
+    ]),
+  ];
+}
+
+main().then(() => process.exit());
diff --git a/scripts/js/commands/checkOrphanPages.ts b/scripts/js/commands/checkOrphanPages.ts
@@ -22,36 +22,19 @@ import { TocEntry } from "../lib/api/generateToc.js";
 
 interface Arguments {
   [x: string]: unknown;
-  currentApis: boolean;
-  devApis: boolean;
-  historicalApis: boolean;
+  apis: boolean;
 }
 
-const ALLOWED_ORPHAN_URLS = new Set([
-  ...qiskitIgnores(),
-  "/api/qiskit/qiskit.primitives.BaseEstimator",
-  "/api/qiskit/qiskit.primitives.BaseSampler",
-]);
+const ALLOWED_ORPHAN_URLS: Set<string> = new Set([...qiskitLegacyIgnores()]);
 
 const readArgs = (): Arguments => {
   return yargs(hideBin(process.argv))
     .version(false)
-    .option("current-apis", {
+    .option("apis", {
       type: "boolean",
       default: false,
       description: "Check the links in the current API docs.",
     })
-    .option("dev-apis", {
-      type: "boolean",
-      default: false,
-      description: "Check the links in the /dev API docs.",
-    })
-    .option("historical-apis", {
-      type: "boolean",
-      default: false,
-      description:
-        "Check the links in the historical API docs, e.g. `api/qiskit/0.44`. ",
-    })
     .parseSync();
 };
 
@@ -60,9 +43,7 @@ async function main() {
 
   const tocFiles = await determineTocFiles(args);
 
-  let allGood = true;
   const orphanPages = [];
-
   for (const tocFile of tocFiles) {
     console.log("Checking toc in:", tocFile);
     const tocUrls = await getTocUrls(tocFile);
@@ -104,16 +85,10 @@ async function collectExistingUrls(directory: string): Promise<string[]> {
 }
 
 async function determineTocFiles(args: Arguments): Promise<string[]> {
-  const globs = ["docs/**/_toc.json", "!docs/api/**"];
-  if (args.currentApis) {
-    globs.push("docs/api/*/_toc.json");
-  }
-  if (args.devApis) {
-    globs.push("docs/api/*/dev/_toc.json");
-  }
-  if (args.historicalApis) {
-    globs.push("docs/api/*/[0-9]*/_toc.json");
-  }
+  const globs = [
+    "docs/**/_toc.json",
+    args.apis ? "docs/api/**/_toc.json" : "!docs/api/**",
+  ];
   return await globby(globs);
 }
 
@@ -131,7 +106,7 @@ function collectTocFileContents(children: TocEntry[]): string[] {
   return urls;
 }
 
-function qiskitIgnores(): string[] {
+function qiskitLegacyIgnores(): string[] {
   const versions = [
     "0.19/",
     "0.24/",
@@ -157,18 +132,13 @@ function qiskitIgnores(): string[] {
     "0.45/",
     "0.46/",
   ];
-
   return [
     ...versions.flatMap((vers) => [
       `/api/qiskit/${vers}aer`,
       `/api/qiskit/${vers}aqua`,
       `/api/qiskit/${vers}ibmq-provider`,
       `/api/qiskit/${vers}ibmq_jupyter`,
       `/api/qiskit/${vers}ibmq_visualization`,
-      `/api/qiskit/${vers}qiskit.aqua.aqua_globals`,
-      `/api/qiskit/${vers}qiskit.optimization.INFINITY`,
-      `/api/qiskit/${vers}qiskit.quantum_info.two_qubit_cnot_decompose`,
-      `/api/qiskit/${vers}qiskit.utils.algorithm_globals`,
       `/api/qiskit/${vers}parallel`,
       `/api/qiskit/${vers}transpiler_builtin_plugins`,
     ]),