[ovs-dev] doc: Don't override default theme

Message ID 20170418102649.10664-1-stephen@that.guru
State Accepted
Headers show

Commit Message

Stephen Finucane April 18, 2017, 10:26 a.m.
Sphinx 1.3 renamed the 'default' theme to 'classic' and configured the
'alabaster' theme as the new default. To prevent breaking existing
builds, the 'default' name was reserved as an alias for 'classic' [1].
However, initially this raised a warning [1] with a message to use
'classic' instead. This warning was removed in 1.3.2 [2], but it will
result in errors (due to the use of the '-W' flag) for Sphinx 1.3.0 and
1.3.1 users.

Mitigate the issue by not setting a theme if the 'ovs_sphinx_theme'
package is absent. This will result in Sphinx using its default theme,
be that 'classic' (Sphinx < 1.3) or 'alabaster'.

[1] https://github.com/sphinx-doc/sphinx/commit/68021b0bd
[2] https://github.com/sphinx-doc/sphinx/commit/034c4e942

Signed-off-by: Stephen Finucane <stephen@that.guru>
Cc: Matthew Thode <mthode@mthode.org>
---
We might want to backport this if there are any releases with the Sphinx
docs present.
---
 Documentation/conf.py | 2 --
 1 file changed, 2 deletions(-)

Comments

Ben Warren via dev April 18, 2017, 2:54 p.m. | #1
On 04/18/2017 05:26 AM, Stephen Finucane wrote:
> Sphinx 1.3 renamed the 'default' theme to 'classic' and configured the
> 'alabaster' theme as the new default. To prevent breaking existing
> builds, the 'default' name was reserved as an alias for 'classic' [1].
> However, initially this raised a warning [1] with a message to use
> 'classic' instead. This warning was removed in 1.3.2 [2], but it will
> result in errors (due to the use of the '-W' flag) for Sphinx 1.3.0 and
> 1.3.1 users.
> 
> Mitigate the issue by not setting a theme if the 'ovs_sphinx_theme'
> package is absent. This will result in Sphinx using its default theme,
> be that 'classic' (Sphinx < 1.3) or 'alabaster'.
> 
> [1] https://github.com/sphinx-doc/sphinx/commit/68021b0bd
> [2] https://github.com/sphinx-doc/sphinx/commit/034c4e942
> 
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Cc: Matthew Thode <mthode@mthode.org>
> ---
> We might want to backport this if there are any releases with the Sphinx
> docs present.
> ---
>  Documentation/conf.py | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 49514ec..97f402e 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -145,8 +145,6 @@ linkcheck_anchors = False
>  #
>  if use_ovs_theme:
>      html_theme = 'ovs'
> -else:
> -    html_theme = 'default'
>  
>  # 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
> 
Thanks for this, looks good and keeps default behaviour by default
instead of hard coding a default.
Ben Pfaff April 21, 2017, 9:33 p.m. | #2
On Tue, Apr 18, 2017 at 09:54:24AM -0500, Matthew Thode via dev wrote:
> On 04/18/2017 05:26 AM, Stephen Finucane wrote:
> > Sphinx 1.3 renamed the 'default' theme to 'classic' and configured the
> > 'alabaster' theme as the new default. To prevent breaking existing
> > builds, the 'default' name was reserved as an alias for 'classic' [1].
> > However, initially this raised a warning [1] with a message to use
> > 'classic' instead. This warning was removed in 1.3.2 [2], but it will
> > result in errors (due to the use of the '-W' flag) for Sphinx 1.3.0 and
> > 1.3.1 users.
> > 
> > Mitigate the issue by not setting a theme if the 'ovs_sphinx_theme'
> > package is absent. This will result in Sphinx using its default theme,
> > be that 'classic' (Sphinx < 1.3) or 'alabaster'.
> > 
> > [1] https://github.com/sphinx-doc/sphinx/commit/68021b0bd
> > [2] https://github.com/sphinx-doc/sphinx/commit/034c4e942
> > 
> > Signed-off-by: Stephen Finucane <stephen@that.guru>
> > Cc: Matthew Thode <mthode@mthode.org>
> > ---
> > We might want to backport this if there are any releases with the Sphinx
> > docs present.
> > ---
> >  Documentation/conf.py | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/Documentation/conf.py b/Documentation/conf.py
> > index 49514ec..97f402e 100644
> > --- a/Documentation/conf.py
> > +++ b/Documentation/conf.py
> > @@ -145,8 +145,6 @@ linkcheck_anchors = False
> >  #
> >  if use_ovs_theme:
> >      html_theme = 'ovs'
> > -else:
> > -    html_theme = 'default'
> >  
> >  # 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
> > 
> Thanks for this, looks good and keeps default behaviour by default
> instead of hard coding a default.

This is a nice way to fix the problem.  Thanks to both of you.

I applied this to master and branch-2.7.

Patch

diff --git a/Documentation/conf.py b/Documentation/conf.py
index 49514ec..97f402e 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -145,8 +145,6 @@  linkcheck_anchors = False
 #
 if use_ovs_theme:
     html_theme = 'ovs'
-else:
-    html_theme = 'default'
 
 # 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