Skip to content

Conversation

Eric-Arellano
Copy link
Collaborator

Part of #495.

--

This PR adds two arguments to npm run check:links, --current-apis and --qiskit-release-notes. In a follow up, we'll add --historical-apis. By default, we don't do any of these checks since it's pretty rare for a PR author to be changing those files and we want npm run check to be as fast as possible locally. CI will still check the whole thing, though.

This makes the script around 2x faster:

npm run check:links  4.04s user 0.32s system 156% cpu 2.797 total
npm run check:links -- --current-apis --qiskit-release-notes  8.66s user 0.99s system 134% cpu 7.182 total

--

To fix the actual broken links, I manually fixed problems in release notes up to 0.44. We are unlikely to regenerate these release notes, so manual fixes should be fine.

I did not fix 0.45, though, since we will continue to regenerate those release notes per #537. We will need to fix the underlying problem in the API docs, or in our API generation script.

@Eric-Arellano Eric-Arellano changed the title [blocked by 557] Check links for release notes Check links for release notes Dec 29, 2023
@Eric-Arellano Eric-Arellano marked this pull request as ready for review December 29, 2023 14:32
Copy link
Collaborator

@arnaucasau arnaucasau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I can't wait to see how this unblocks the validation of historical versions!

Comment on lines +144 to +146
# By default, only the non-API docs are checked. You can add any of these
# options to check API docs.
npm run check:links -- --current-apis --qiskit-release-notes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Mention the Qiskit release notes in the comment to avoid thinking they need each other to work, or maybe add a different one similar to the external links

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if this more clear in the follow up.

@Eric-Arellano Eric-Arellano added this pull request to the merge queue Dec 29, 2023
Merged via the queue into main with commit 304eaf6 Dec 29, 2023
@Eric-Arellano Eric-Arellano deleted the EA/check-release-notes branch December 29, 2023 16:01
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Part of Qiskit#495. 

--

This PR adds two arguments to `npm run check:links`, `--current-apis`
and `--qiskit-release-notes`. In a follow up, we'll add
`--historical-apis`. By default, we don't do any of these checks since
it's pretty rare for a PR author to be changing those files and we want
`npm run check` to be as fast as possible locally. CI will still check
the whole thing, though.

This makes the script around 2x faster:

```
npm run check:links  4.04s user 0.32s system 156% cpu 2.797 total
npm run check:links -- --current-apis --qiskit-release-notes  8.66s user 0.99s system 134% cpu 7.182 total
```

--

To fix the actual broken links, I manually fixed problems in release
notes up to 0.44. We are unlikely to regenerate these release notes, so
manual fixes should be fine.

I did not fix 0.45, though, since we will continue to regenerate those
release notes per Qiskit#537. We
will need to fix the underlying problem in the API docs, or in our API
generation script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants