On 11/6/23 17:06, Mark Michelson wrote: > Hi Ilya, > > Thanks for fixing this. > > Acked-by: Mark Michelson <[email protected]> > > I have one suggestion below. > > On 11/2/23 12:15, Ilya Maximets wrote: >> 'ovs-sphinx-theme' is designed to look like an openvswitch.org >> website. It contains OVS logo and navigation bars from the >> openvswitch.org. And that doesn't really make a lot of sense >> for OVN. Also, currently the ovs-sphinx-theme is not actually >> installed by the Read The Docs configuration, so the docs.ovn.org >> is using default alabaster theme instead. >> >> Switch to sphinx_rtd_theme, it looks close to the main ovn.org >> website. Remove the upper limit on sphinx version, because the >> theme may require higher versions and also sphinx 2.0 is very old >> and fails to be installed on Read The Docs servers. >> >> The import in conf.py will not be actually used, importing only >> to check availability, so ignoring the F401 'imported but unused'. >> >> Signed-off-by: Ilya Maximets <[email protected]> >> --- >> Documentation/conf.py | 20 +++++++------------- >> Documentation/internals/documentation.rst | 18 +++++++----------- >> Documentation/requirements.txt | 4 ++-- >> 3 files changed, 16 insertions(+), 26 deletions(-) >> >> diff --git a/Documentation/conf.py b/Documentation/conf.py >> index f7eceaec8..5cd0041b8 100644 >> --- a/Documentation/conf.py >> +++ b/Documentation/conf.py >> @@ -16,12 +16,12 @@ import string >> import sys >> >> try: >> - import ovs_sphinx_theme >> - use_ovs_theme = True >> + import sphinx_rtd_theme # noqa: F401 >> + use_rtd_theme = True >> except ImportError: >> - print("Cannot find 'ovs-sphinx-theme' package. " >> + print("Cannot find 'sphinx_rtd_theme' package. " >> "Falling back to default theme.") >> - use_ovs_theme = False >> + use_rtd_theme = False > > We can avoid the unused import by using importlib. > > import importlib > > use_rtd_theme = importlib.util.find_spec('sphinx_rtd_theme') is not None > if not use_rtd_theme: > print("Cannot find 'sphinx_rtd_theme' package. " > "Falling back to default theme.") > > Is this easier to understand than the existing code? That's questionable > . However, we would get rid of the "# noqa" code smell doing it this way.
Makes sense. I'll send v2. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
