Skip to content

Conversation

@jablonskidev
Copy link
Contributor

This should fix issues #652 and #653 in the devguide. (The issues are duplicates of each other, so this PR is still addressing only one issue at a time.)

Co-authored-by: CAM Gerlach <CAM.Gerlach@Gerlach.CAM>
@jablonskidev
Copy link
ContributorAuthor

@CAM-Gerlach Thanks again, that was super helpful, and I learned more about Sphinx along the way. I committed your second suggestion and will also submit a PR to add the license URL to the dictionary html_theme_options inside the devguide's conf.py.

@CAM-Gerlach
Copy link
Member

You're welcome, I'm glad! I'm no expert at Sphinx theming either; I know just enough to be dangerous 😆 Looks like its up to @Mariatta for the final review?

@matrixise
Copy link
Member

Could you explain how to use it? because with my workflow, it doesn't work.

git clone https://github.com/python/devguide cd devguide make venv ./venv/bin/pip install -U pip ./venv/bin/pip uninstall python-docs-theme ./venv/bin/pip install git+https://github.com/jablonskidev/python-docs-theme.git@patch-1 

patch with this diff

diff --git a/conf.py b/conf.py index 4071095..f20b61f 100644 --- a/conf.py+++ b/conf.py@@ -95,6 +95,7 @@ pygments_style = 'sphinx' # Use the upstream python-docs-theme html_theme = 'python_docs_theme' html_theme_options ={+ 'copyright_url': 'https://www.demo.org', 'collapsiblesidebar': True, 'issues_url': 'https://github.com/python/devguide/issues/new', }
> make html ./venv/bin/sphinx-build -b html -d _build/doctrees . _build/html Running Sphinx v3.5.4 making output directory... done loading intersphinx inventory from https://docs.python.org/3/objects.inv... building [mo]: targets for 0 po files that are out of date building [html]: targets for 35 source files that are out of date updating environment: [new config] 35 added, 0 changed, 0 removed reading sources... [100%] triaging looking for now-outdated files... none found pickling environment... done checking consistency... done preparing documents... WARNING: unsupported theme option 'copyright_url' given done writing output... [100%] triaging generating indices... genindex done writing additional pages... search done copying images... [100%] images/python-cyclic-gc-5-new-page.png copying static files... done copying extra files... done dumping search index in English (code: en)... done dumping object inventory... done build succeeded, 1 warning. The HTML pages are in _build/html. Build finished. The HTML pages are in _build/html. 

As you can see, I have got WARNING: unsupported theme option 'copyright_url' given

Any idea?

Thank you

@CAM-Gerlach
Copy link
Member

Thanks @matrixise . This PR neglected to add

copyright_url = 

in theme.conf as mentioned in my review comment above, and it seems I didn't notice the omission earlier, sorry. That's why you're seeing the warning. However, it may still have worked, I'm not certain—I presume you actually checked the build output and the link was not correct?

@matrixise
Copy link
Member

That's why you're seeing the warning. However, it may still have worked, I'm not certain—I presume you actually checked the build output and the link was not correct?

In fact, without this parameter, the build of the devguide or another project using this PR would generate a Warning with the undefined copyright_url option

In this case, the copyright_url must be defined into the theme.conf file.

@jablonskidev Could you update your PR with the copyright_url option?

Thank you

@matrixisematrixise self-assigned this May 6, 2021
@jablonskidev
Copy link
ContributorAuthor

@matrixise@CAM-Gerlach Thanks for your feedback! I added copyright_url = to theme.conf. If there's anything else I can do to improve this and help move it forward, then please let me know.

@matrixise
Copy link
Member

Thanks @jablonskidev I am going to check in few minutes.

@matrixise
Copy link
Member

@Mariatta What do you think about this PR, for me, it's fine, and we can merge it.

@matrixise
Copy link
Member

@JulienPalard Just to be sure. For you, is there an issue with this PR and the "copyright" link in the existing documentations? Thanks

Copy link
Member

@matrixisematrixise left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution.

@matrixisematrixise merged commit 6ed468a into python:masterMay 25, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@jablonskidev@CAM-Gerlach@matrixise@JulienPalard@Mariatta@the-knights-who-say-ni