Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release notes for historical versions should link to stable API docs #755

Closed
Tracked by #479
Eric-Arellano opened this issue Feb 2, 2024 · 1 comment
Closed
Tracked by #479

Comments

@Eric-Arellano
Copy link
Collaborator

Qiskit release notes before Qiskit 0.45 were stored in the file legacy_release_notes.rst, which was deleted upstream. This means that it's safe for us to make manual changes to release notes for Qiskit before v0.45, which we've been doing without issue.

But starting with Qiskit 0.45, the release notes are included in release_notes.rst. This means that generating the release notes for 1.0 will regenerate the release notes for 0.45. So we can't make manual changes anymore!

Not being able to make manual changes is a problem because we sometimes have to fix things like broken links (see 427efcc and 22b0591). But then those manual changes get reverted next time we regenerate the docs, like ba0fb97.

I'm pretty sure the correct solution is for us to fix the original source code, like the Reno release notes folder in Qiskit. We need to fix the broken links to stable links.

Eric-Arellano added a commit that referenced this issue Feb 6, 2024
Reverts #718. Turns out that
breaks certain pages, as explained at
#718 (comment).

Instead, we fix the problematic pages by changing the original Sphinx
HTML in Box directly in this PR's last commit.

We have to manually revert the 0.45 docs due to
#755.
@Eric-Arellano Eric-Arellano self-assigned this Feb 14, 2024
@Eric-Arellano Eric-Arellano moved this to In Progress in Docs Planning Feb 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 14, 2024
#804)

When Qiskit 1.0 is released, a bunch of APIs will be removed from our
Qiskit API docs. This breaks internal links we have.

This PR switches all links that will break to point to Qiskit 0.44. Once
1.0 is released, we should switch what we can to 0.46, per
#803.

It does not change Qiskit release notes for 0.45 and 0.46, since we
can't safely make manual changes to those, per
#755. Those still need to
be dealt with.

## How change was generated

I found the issues by generating 1.0 docs as the current version, then
running `npm run check:links -- --qiskit-release-notes`. I started with
manual fixes, then realized this is easily automatable.

I accidentally `git clean`ed the script, but it looked like this:

```python
from pathlib import Path
from typing import DefaultDict
from collections import defaultdict
import re


def parse_error(error: str, file_to_bad_links: DefaultDict[str, list[str]]) -> None:
    lines = error.splitlines()
    bad_link_line = lines[0]
    file_list_lines = lines[1:]

    bad_link = ""  # Regex that captured the link
    files = [line.split("❓")[0].strip() for line in file_list_lines]

    for file in files:
        file_to_bad_links[file].append(bad_link)


def read_errors() -> DefaultDict[str, list[str]]:
    raw_text = Path("api_errors.txt").read_text()
    file_to_bad_links = defaultdict(list)
    for error in raw_text.split("❌"):
        parse_error(error, file_to_bad_links)
    return file_to_bad_links


def rewrite_file(fp: str, bad_links: list[str]) -> None:
   # code to use `string.replace()`
   pass 

def main() -> None:
    file_to_bad_links = read_errors()
    for file, bad_links in file_to_bad_links.items():
      rewrite_file(file, bad_links)


main()
```

Then I had to fix a few links manually that were only in 0.45 and not
0.44.
@Eric-Arellano Eric-Arellano removed their assignment Feb 14, 2024
@Eric-Arellano Eric-Arellano changed the title Figure out how to make changes to Qiskit 0.45+ release notes Release notes for historical versions should link to stable API docs Feb 20, 2024
@Eric-Arellano
Copy link
Collaborator Author

But starting with Qiskit 0.45, the release notes are included in release_notes.rst. This means that generating the release notes for 1.0 will regenerate the release notes for 0.45. So we can't make manual changes anymore!

We decided in Qiskit/qiskit#11840 to change this. Now the version should only change for its own release note. Meaning 1.0 should only change 1.0.md, not 0.45.md and 0.46.md.

I'm curious if we can simplify our logic for release notes thanks to this change.

--

But, we still have the problem of broken links because historical API docs will use /api/qiskit/ links by default, meaning they point to the current API docs rather than historical API docs. Instead, we should point to /api/qiskit/0.45 for example.

We should change this line if it's historical API docs:

if (pkg.hasSeparateReleaseNotes && path.endsWith("release-notes.md")) {
// Convert the relative links to absolute links
result.markdown = transformLinks(result.markdown, (link, _) =>
link.startsWith("http") || link.startsWith("#") || link.startsWith("/")
? link
: `/api/${pkg.name}/${link}`,
);

--

We might want to change our link checker behavior in preparation for this. We need the link checker to load all the API docs for the corresponding release note, like 0.45.md needs access to all of 0.45.md. Currently, we hardcode the files to load when checking release notes here:

// The 0.46 docs are used by release notes for APIs that were removed in 1.0.
"docs/api/qiskit/0.46/*.md",
"docs/api/qiskit/0.44/qiskit.extensions.{Hamiltonian,Unitary}Gate.md",
"docs/api/qiskit/0.45/qiskit.quantum_info.{OneQubitEuler,TwoQubitBasis,XX}Decomposer.md",
"docs/api/qiskit/0.45/qiskit.transpiler.synthesis.aqc.AQC.md",
"docs/api/qiskit/0.45/{tools,quantum_info,synthesis_aqc}.md",

Instead, maybe we should for Qiskit 0.45+ have a distinct FileBatch per release note, like load all of 0.45 and check 0.45.md in a single FileBatch; then 0.46 is its own FileBatch.

This optimization doesn't need to block though. We can make a dedicated issue for it.

@arnaucasau arnaucasau self-assigned this Feb 20, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 22, 2024
Part of #755 

This PR changes how we are checking the links of the Qiskit release
notes. The link checker will check every file in a different batch
allowing us to load all the necessary files for each release note.

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this issue Feb 23, 2024
…#860)

### Summary 

Part of #755

This PR changes the logic of the API generation script to only modify
the release notes file of the version we are regenerating at that
moment. Before, the release notes files contained more than one version,
and we needed to update several files, independently of what version we
were regenerating. After Qiskit/qiskit#11840, we
can assume that the release notes files will only contain their own
versions, and we can simply our script by removing some functions.

### New logic

The API generation script will transform every link of the release notes
files to point to its version folder instead of the top level. Once we
have the correct links, we will directly write the release notes file,
if we didn't have them before, or otherwise, we will create a new file
containing the header of the old file we had and the version sections of
the new one downloaded from Box. That way, we can make manual changes
like the table added in the release notes of qiskit 0.44 without losing
them in the next regeneration.

### Functions removed

This change allows us to remove the following two functions:

The `extractMarkdownReleaseNotesPatches` function extracted all the
versions in a release notes file and stored the markdown of each patch
to posteriorly merge them under their minor version file. Now we have
one minor version per file, so we don't need to break the release notes
into pieces anymore. The new logic treats all patch versions as a block.

The `sortReleaseNotesVersions` were used to sort the patch versions.
Given that the file will have the correct order, we don't need to worry
about it either.

Removing these two functions allows us to remove the test file, given
that they composed the entire file.
@Eric-Arellano Eric-Arellano moved this from In Progress to In Review in Docs Planning Feb 26, 2024
Eric-Arellano pushed a commit that referenced this issue Feb 26, 2024
Part of #755

This PR changes the legacy release notes links to point to the qiskit
0.45 version folder.

### How the change was done

I created a little script to transform every link in a release note file
and re-write it in the same destination. I named it `updateLinks.ts`.

```ts
import { zxMain } from "../lib/zx";
import { globby } from "globby";
import { pathExists, getRoot } from "../lib/fs";
import { readFile, writeFile } from "fs/promises";
import transformLinks from "transform-markdown-links";

zxMain(async () => {
  const allVersions = (
    await globby("docs/api/qiskit/release-notes/[!index]*")
  ).map((releaseNotesPath) =>
    releaseNotesPath.split("/").pop()!.split(".").slice(0, -1).join("."),
  );

  for (const releaseNotesVersion of allVersions) {
    const source = `${getRoot()}/docs/api/qiskit/release-notes/${releaseNotesVersion}.md`;
    if (await pathExists(source)) {
      updateLinksFile('qiskit', releaseNotesVersion, source, source);
    }
  }
});

async function updateLinksFile(pkgName: string, versionWithoutPatch: string, source: string, dest: string) {
  let markdown = await readFile(source, { encoding: "utf8" });

  const regexAbsolutePath = new RegExp(
    "/api/" + pkgName + "/(?!release-notes)(?![0-9])",
  );

  markdown = transformLinks(markdown, (link, _) =>
    link.replace(regexAbsolutePath, `/api/${pkgName}/0.45/`),
  );

  await writeFile(dest, markdown);
}


```

To execute the script using `npm run update-legacy-links` you can modify
the `package.json` file by adding the following line in the `scripts`
section:

```ts
"update-legacy-links": "node -r esbuild-register scripts/commands/updateLinks.ts"
```
@github-project-automation github-project-automation bot moved this from In Review to Done in Docs Planning Feb 28, 2024
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Reverts Qiskit#718. Turns out that
breaks certain pages, as explained at
Qiskit#718 (comment).

Instead, we fix the problematic pages by changing the original Sphinx
HTML in Box directly in this PR's last commit.

We have to manually revert the 0.45 docs due to
Qiskit#755.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Qiskit#804)

When Qiskit 1.0 is released, a bunch of APIs will be removed from our
Qiskit API docs. This breaks internal links we have.

This PR switches all links that will break to point to Qiskit 0.44. Once
1.0 is released, we should switch what we can to 0.46, per
Qiskit#803.

It does not change Qiskit release notes for 0.45 and 0.46, since we
can't safely make manual changes to those, per
Qiskit#755. Those still need to
be dealt with.

## How change was generated

I found the issues by generating 1.0 docs as the current version, then
running `npm run check:links -- --qiskit-release-notes`. I started with
manual fixes, then realized this is easily automatable.

I accidentally `git clean`ed the script, but it looked like this:

```python
from pathlib import Path
from typing import DefaultDict
from collections import defaultdict
import re


def parse_error(error: str, file_to_bad_links: DefaultDict[str, list[str]]) -> None:
    lines = error.splitlines()
    bad_link_line = lines[0]
    file_list_lines = lines[1:]

    bad_link = ""  # Regex that captured the link
    files = [line.split("❓")[0].strip() for line in file_list_lines]

    for file in files:
        file_to_bad_links[file].append(bad_link)


def read_errors() -> DefaultDict[str, list[str]]:
    raw_text = Path("api_errors.txt").read_text()
    file_to_bad_links = defaultdict(list)
    for error in raw_text.split("❌"):
        parse_error(error, file_to_bad_links)
    return file_to_bad_links


def rewrite_file(fp: str, bad_links: list[str]) -> None:
   # code to use `string.replace()`
   pass 

def main() -> None:
    file_to_bad_links = read_errors()
    for file, bad_links in file_to_bad_links.items():
      rewrite_file(file, bad_links)


main()
```

Then I had to fix a few links manually that were only in 0.45 and not
0.44.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Part of Qiskit#755 

This PR changes how we are checking the links of the Qiskit release
notes. The link checker will check every file in a different batch
allowing us to load all the necessary files for each release note.

---------

Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
…Qiskit#860)

### Summary 

Part of Qiskit#755

This PR changes the logic of the API generation script to only modify
the release notes file of the version we are regenerating at that
moment. Before, the release notes files contained more than one version,
and we needed to update several files, independently of what version we
were regenerating. After Qiskit/qiskit#11840, we
can assume that the release notes files will only contain their own
versions, and we can simply our script by removing some functions.

### New logic

The API generation script will transform every link of the release notes
files to point to its version folder instead of the top level. Once we
have the correct links, we will directly write the release notes file,
if we didn't have them before, or otherwise, we will create a new file
containing the header of the old file we had and the version sections of
the new one downloaded from Box. That way, we can make manual changes
like the table added in the release notes of qiskit 0.44 without losing
them in the next regeneration.

### Functions removed

This change allows us to remove the following two functions:

The `extractMarkdownReleaseNotesPatches` function extracted all the
versions in a release notes file and stored the markdown of each patch
to posteriorly merge them under their minor version file. Now we have
one minor version per file, so we don't need to break the release notes
into pieces anymore. The new logic treats all patch versions as a block.

The `sortReleaseNotesVersions` were used to sort the patch versions.
Given that the file will have the correct order, we don't need to worry
about it either.

Removing these two functions allows us to remove the test file, given
that they composed the entire file.
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Part of Qiskit#755

This PR changes the legacy release notes links to point to the qiskit
0.45 version folder.

### How the change was done

I created a little script to transform every link in a release note file
and re-write it in the same destination. I named it `updateLinks.ts`.

```ts
import { zxMain } from "../lib/zx";
import { globby } from "globby";
import { pathExists, getRoot } from "../lib/fs";
import { readFile, writeFile } from "fs/promises";
import transformLinks from "transform-markdown-links";

zxMain(async () => {
  const allVersions = (
    await globby("docs/api/qiskit/release-notes/[!index]*")
  ).map((releaseNotesPath) =>
    releaseNotesPath.split("/").pop()!.split(".").slice(0, -1).join("."),
  );

  for (const releaseNotesVersion of allVersions) {
    const source = `${getRoot()}/docs/api/qiskit/release-notes/${releaseNotesVersion}.md`;
    if (await pathExists(source)) {
      updateLinksFile('qiskit', releaseNotesVersion, source, source);
    }
  }
});

async function updateLinksFile(pkgName: string, versionWithoutPatch: string, source: string, dest: string) {
  let markdown = await readFile(source, { encoding: "utf8" });

  const regexAbsolutePath = new RegExp(
    "/api/" + pkgName + "/(?!release-notes)(?![0-9])",
  );

  markdown = transformLinks(markdown, (link, _) =>
    link.replace(regexAbsolutePath, `/api/${pkgName}/0.45/`),
  );

  await writeFile(dest, markdown);
}


```

To execute the script using `npm run update-legacy-links` you can modify
the `package.json` file by adding the following line in the `scripts`
section:

```ts
"update-legacy-links": "node -r esbuild-register scripts/commands/updateLinks.ts"
```
@javabster javabster removed this from Docs Planning Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants