John Snow <js...@redhat.com> writes: > On 1/20/21 7:07 AM, Markus Armbruster wrote: >> John Snow <js...@redhat.com> writes: [...] >>> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py >>> index e03abcbb959..f754f675d66 100644 >>> --- a/docs/sphinx/qapidoc.py >>> +++ b/docs/sphinx/qapidoc.py >>> @@ -463,11 +463,11 @@ def __init__(self, env, qapidir): >>> self._env = env >>> self._qapidir = qapidir >>> - def visit_module(self, name): >>> - if name is not None: >>> - qapifile = self._qapidir + '/' + name >>> + def visit_module(self, module): >>> + if module.name: >> Replacing the "is not None" test by (implicit) "is thruthy" changes >> behavior for the empty string. Intentional? >> > > Instinctively it was intentional, consciously it wasn't. I was worried > about what "qapifile" would produce if the string happened to be > empty.
It would produce a dependency on the directory. >> I've had the "pleasure" of debugging empty strings getting interpreted >> like None where they should be interpreted like any other string. >> > > assert module.name, then? Let's avoid changing behavior in a refactoring patch. If you want an assertion to ease your worry, separate patch. >>> + qapifile = self._qapidir + '/' + module.name >>> self._env.note_dependency(os.path.abspath(qapifile)) >>> - super().visit_module(name) >>> + super().visit_module(module) >>> >> [...] >>
