-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix ReadTheDocs PDF build #64
Conversation
RTD is being a real pain. So, the issue was bound to RTD using the root of the git clone and then generating files in
I really thought RTD was just ignoring some files listed in |
…hs, and adding cleanup steps
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.
LGTM, there are two nitpicks I would like to be addressed before merging.
note: I can not access https://canonical-test-starter-pack-pdfgen.readthedocs-hosted.com/en/rtd-subdir/ and the PR preview does not show a download as PDF link, but I trust you that it works now.
note: The link check errors are all expected and should not block the merge. They are fixed in #53 (not merged yet).
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.
You can try https://canonical-test-starter-pack-pdfgen.readthedocs-hosted.com/_/sharing/G3ZK1L59yDc9fFutCcA3kuZ5Lr7C8J1L?next=/en/rtd-subdir/ to check the build preview - I'm not sure what level of access the temporary sharing URL provides though.
Apply suggested changes Co-authored-by: Dominik Viererbe <[email protected]>
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.
You can try https://canonical-test-starter-pack-pdfgen.readthedocs-hosted.com/_/sharing/G3ZK1L59yDc9fFutCcA3kuZ5Lr7C8J1L?next=/en/rtd-subdir/ to check the build preview - I'm not sure what level of access the temporary sharing URL provides though.
This works for me now and I could verify that the download PDF link works :)
Merging now, thanks again for fixing this!
Fixes issue outlined in #58