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

Pages render check only runs on changed files #913

Merged
merged 6 commits into from
Feb 28, 2024

Conversation

Eric-Arellano
Copy link
Collaborator

Closes #886. As explained there, it's safe to only run this check on changed files because whether a page renders is independent from all the other pages.

We use a weekly cron job, though, to check all pages. That reduces our risk from things like the docs preview Docker image changing on us and resulting in some pages now failing to render.

--

This PR also stops checking external links in API docs builds and instead moves the check to the new weekly cron job. Relates to #876.

Per #372, it's annoying to check external links in CI because it can be flaky (#823).

@Eric-Arellano Eric-Arellano changed the title [wip] Pages render check only runs on changed files Pages render check only runs on changed files Feb 27, 2024
@Eric-Arellano Eric-Arellano marked this pull request as ready for review February 27, 2024 22:46
Copy link
Collaborator Author

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Code looks good and works well 🚀

run: npm run check-pages-render
if: steps.changed-docs-files.outputs.any_changed == 'true'
run: |
echo "${{ steps.changed-docs-files.outputs.all_changed_files }}" > changed.txt
Copy link
Member

Choose a reason for hiding this comment

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

Nice solution

@Eric-Arellano Eric-Arellano added this pull request to the merge queue Feb 28, 2024
Merged via the queue into main with commit 927b2b5 Feb 28, 2024
2 checks passed
@Eric-Arellano Eric-Arellano deleted the EA/pages-render-workflow branch February 28, 2024 12:44
github-merge-queue bot pushed a commit that referenced this pull request Feb 28, 2024
Closes #876.

This split brings several improvements:

1. Simpler internal link checker code.
2. The external link checker now deduplicates links across all files,
which avoids repeating requests.
3. The external link checker logs its progress every 20 links.
4. It's possible to check external links without checking internal
links.

We run the external link checker in our weekly cron job over all files,
other than translations and historical Qiskit versions. That change was
added in #913.
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
Closes Qiskit#886. As explained
there, it's safe to only run this check on changed files because whether
a page renders is independent from all the other pages.

We use a weekly cron job, though, to check all pages. That reduces our
risk from things like the docs preview Docker image changing on us and
resulting in some pages now failing to render.

--

This PR also stops checking external links in API docs builds and
instead moves the check to the new weekly cron job. Relates to
Qiskit#876.

Per Qiskit#372, it's annoying to
check external links in CI because it can be flaky
(Qiskit#823).
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
…it#916)

Closes Qiskit#876.

This split brings several improvements:

1. Simpler internal link checker code.
2. The external link checker now deduplicates links across all files,
which avoids repeating requests.
3. The external link checker logs its progress every 20 links.
4. It's possible to check external links without checking internal
links.

We run the external link checker in our weekly cron job over all files,
other than translations and historical Qiskit versions. That change was
added in Qiskit#913.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Only run pages render check on changed files
2 participants