curtin-dev team mailing list archive

[Dan Bungert <mp+441502@xxxxxxxxxxxxxxxxxx>]

Thanks for sending this along, having the curtin docs match other things is a nice improvements.  I have some small tweaks to request.

Diff comments:

> diff --git a/doc/conf.py b/doc/conf.py
> index d7a38ff..fc0cd34 100644
> --- a/doc/conf.py
> +++ b/doc/conf.py
> @@ -96,19 +96,15 @@ pygments_style = 'sphinx'
>  
>  # The theme to use for HTML and HTML Help pages.  See the documentation for
>  # a list of builtin themes.
> -html_theme = 'classic'
> +#html_theme = 'classic'
>  
>  # on_rtd is whether we are on readthedocs.org, this line of code grabbed from
>  # docs.readthedocs.org
> -on_rtd = os.environ.get('READTHEDOCS', None) == 'True'
> +#on_rtd = os.environ.get('READTHEDOCS', None) == 'True'

Let's remove unused code, and the matching comments.

>  
> -if not on_rtd:  # only import and set the theme if we're building docs locally
> -    import sphinx_rtd_theme
> -    html_theme = 'sphinx_rtd_theme'
> -    html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
> -
> -# otherwise, readthedocs.org uses their theme by default, so no need to specify
> -# it
> +import sphinx_rtd_theme

We prefer to move the imports to the top, let's do so here.  It made sense to have it in the conditional before before but now the late import looks odd.

> +html_theme = 'sphinx_rtd_theme'

Since there is an existing html_theme bit, moving these two up to currently commented out 'classic' line should keep it simpler.

> +html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
>  
>  # Theme options are theme-specific and customize the look and feel of a theme
>  # further.  For a list of options available for each theme, see the


-- 
https://code.launchpad.net/~virtuous-sloth/curtin/+git/curtin/+merge/441502
Your team curtin developers is requested to review the proposed merge of ~virtuous-sloth/curtin:fix-theme-and-toc into curtin:master.