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

docs: use pytket-docs-theming submodule #153

Merged
merged 45 commits into from
Oct 2, 2024
Merged

Conversation

CalMacCQ
Copy link
Contributor

@CalMacCQ CalMacCQ commented Aug 27, 2024

Description

Updating the docs build to use the pytket-docs-theming repository as a submodule. This means the docs can be built locally with the latest theme config.

I also took great pleasure in deleting a bunch of legacy docs files ;). If we go with this solution then we don't these anymore.

Take a look at the README for building the docs locally -> https://github.com/CQCL/pytket-docs-theming/tree/main/extensions#building-the-api-docs-for-pytket-extensions

I've added some bash scripts to automate installing dependencies and building the docs. Note that these scripts are only intended for local development and will not be used in the website build. I don't want to have to make sure that they match in all of the different repositories.

Screenshot 2024-09-04 at 01 27 54

TODOS

  • Finalise how docs dependencies are specified. (proposal added in README) Happy to have input on this
  • Fix docs build in C.I.
  • Discuss whether this is feasible for all other extenions that we care about.

Related issues

Please mention any github issues addressed by this PR.

Copy link
Collaborator

@PabloAndresCQ PabloAndresCQ left a comment

Choose a reason for hiding this comment

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

Awesome! This is just what I needed. I have been able to build the docs locally following the instructions in docs/README.md. This will allow me to check the docs are built correctly before merging branches to main. Thanks!

I'll wait for you to chat with Melf about the remaining details. Once that's done, ping me again (or ask Melf to accept it). LGTM!

Copy link
Collaborator

@cqc-melf cqc-melf left a comment

Choose a reason for hiding this comment

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

This looks good to me.
For me this opens the question if we want to update the other extensions to use this, too?

@CalMacCQ CalMacCQ changed the title docs: Use pytket-docs-theming [DEMO] docs: use pytket-docs-theming submodule Oct 1, 2024
@CalMacCQ
Copy link
Contributor Author

CalMacCQ commented Oct 1, 2024

The build and test job seems to be failing. Its annoying as I've made this pass for other extensions...

Any ideas @cqc-melf ? I've made it work on pytket-qiskit with no issues.

I think we should just have one docs build in C.I. if possible.

@CalMacCQ CalMacCQ removed the request for review from PabloAndresCQ October 2, 2024 10:00
@cqc-melf
Copy link
Collaborator

cqc-melf commented Oct 2, 2024

@PabloAndresCQ do you want to take a look, I would be happy to approve this now.

@PabloAndresCQ PabloAndresCQ self-requested a review October 2, 2024 14:07
Copy link
Collaborator

@PabloAndresCQ PabloAndresCQ left a comment

Choose a reason for hiding this comment

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

I've had a quick look at the diff, I don't fully understand what's going on, but none of this seems to affect anything other than doc building, so it looks OK to me.

I have once again built the docs locally using the instructions from the README, and everything worked flawlessly. I'm keen on getting this merged :)

@PabloAndresCQ
Copy link
Collaborator

Thanks both! This is very useful

@CalMacCQ CalMacCQ merged commit e61af70 into main Oct 2, 2024
9 checks passed
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.

3 participants