-
Notifications
You must be signed in to change notification settings - Fork 88
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
Ensure API docs don't have stale images #2541
Conversation
} else if (pkg.isDev()) { | ||
// We don't want to store images from other versions when we generate a | ||
// different dev version | ||
await rmFilesInFolder(destFolder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary because updateApiDocs.ts will have already removed all images as part of removing Markdown.
Hold off on merging.
I need to figure out the cleanest way to fix this problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!! Looking at the lost images, they were manually added to the current version folder in #346. Given that only those two versions use the legacy release note images, maybe we could store the images in their folders and manually modify their links in 0.40.mdx
and 0.43.mdx
to point to the new location.
Thanks for finding that. I tried that approach but we still don't copy the images because this code only copies images we actually use: documentation/scripts/js/lib/api/conversionPipeline.ts Lines 138 to 143 in acdcf5b
So, I think the best approach is to instead save the images in a non-auto-generated folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Tested locally and works well! 🚀
Closes #2538. We only didn't delete the API images due to #2540.
To fix #2540, we have to fix our skip of release note images. We also had to manually move 4 release note images for legacy release note files from the latest Qiskit images folder to instead the 0.40 and 0.43 Box artifacts, then manually update those release notes to point to the paths. With those fixes and the re-uploaded Box artifacts,
npm run regen-apis
doesn't make changes.This change allows us to have checkStaleImages.ts no longer worry about API docs images, which dramatically speeds up the program. It's now fast enough to run on every PR. Closes #2530.
With ripgrep:
With git grep (CI):