Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Create new documentation page #1166

Merged
merged 26 commits into from
May 11, 2023

Conversation

Guillermo-Mijares-Vilarino
Copy link
Contributor

@Guillermo-Mijares-Vilarino Guillermo-Mijares-Vilarino commented Apr 10, 2023

Summary

This PR aims to create a separate documentation page for this repo.

Details and comments

  • Change sphinx_rtd_theme to qiskit_sphinx_theme
  • Add the tutorials to the sidebar.
  • Use nbsphinx to render the tutorials.
  • Create workflow for docs deployment to qiskit.org/ecosystem

TODO:

  • Add rclone keys

@Guillermo-Mijares-Vilarino Guillermo-Mijares-Vilarino changed the title Create doc page Create new documentation page Apr 10, 2023
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
@1ucian0 1ucian0 marked this pull request as ready for review April 12, 2023 13:34
QISKIT_PARALLEL: False
QISKIT_DOCS_BUILD_TUTORIALS: 'always'
run: |
echo "earliest_version: 0.1.0" >> releasenotes/config.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Why not just put this in the reno configuration directly config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something I took from several of the applications repos (see these PRs on nature, optimization and machine learning).

However, I see that qiskit-aer and qiskit-dynamics don't include any reference to earliest_version (in neither their respective releasenotes/config.yaml nor the docs deployment workflow) and seem to work just fine.

Is it possible that earliest_version is something that was necessary 2 years ago (when the applications PRs were done) but not now? Maybe I can simply remove that line with no ill effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just removed that line. Is there any way to test whether the deployment process would be successful @mtreinish?

Copy link
Member

Choose a reason for hiding this comment

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

There isn't really a test mechanism for the deployment process. We either publish it or not, the fundamental limitation comes from access to secrets, for security reasons the secrets aren't available to CI run from forks to limit access to only reviewed/merged code.

The only option to test this is post merge to click the button for the workflow dispatch in the UI. This is probably generally advisable anyway because there haven't really been any changes to the ibmq provider to trigger a release so we can just do a manual build from the stable branch.

@HuangJunye
Copy link

@1ucian0 Can you please assign me so that I can keep track of the PR review? Thanks!

Copy link

@HuangJunye HuangJunye left a comment

Choose a reason for hiding this comment

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

LGTM, but I can't test the deployment. @mtreinish Is there a way to test the deployment before merging? I think we add a temporary trigger for the deploy-docs workflow to be triggered on PR. The workflow requires secrets which I think will only work if we make a PR from a branch of the upstream repo instead of a fork. Should we do that here?

docs/conf.py Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
requirements-dev.txt Outdated Show resolved Hide resolved
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Overall this LGTM just a couple small things inline. Also it would be good while we're updating this to add -d {toxinidir}/docs/.doctrees to the tox command for the docs build so that we're not publishing the sphinx build cache to COS. This will make the upload faster, save bandwidth and disk space too.

docs/conf.py Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@Guillermo-Mijares-Vilarino
Copy link
Contributor Author

I've just added the -d {toxinidir}/docs/.doctrees to docs build and included the docs/.doctrees/ directory as part of .gitignore.

With this I believe all the comments are addressed.

@mtreinish mtreinish merged commit 06d6e50 into Qiskit:master May 11, 2023
11 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants