Closed Bug 920638 Opened 8 years ago Closed 8 years ago

Integrate symbols into Sphinx documentation


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: gps, Assigned: gps)



(1 file)

The docstrings in describing the symbols in the sandbox are restructured text. Now that we have Sphinx, we should find a way to integrate them into the Sphinx docs.

FWIW, the reason there is a test to cover docstring formatting is to ensure the docstrings can be formatted consistently without breaking |mach mozbuild-reference| and other downstream consumers relying on them being valid restructured text.
This should mostly be a rubber stamp review. But, there are a few
changes in python/mozbuild that warrant review.

Essentially this patch does 3 things:

1) Adds an --api-docs mode to |mach build-docs| which will generate .rst
API docs for Mozilla's in-tree Python packages
2) Adds newly generated .rst files to version control.
3) Integrates the mozbuild sandbox symbols into Sphinx

#3 is the most complicated. I had to write a Sphinx extension to import
the symbols. I could have introduced a manual step where we need to
update an .rst file every time we update, but that's
tedious and won't get done. The extension turned out to be rather

There's lots of moving around code from the mozbuild-reference mach
command module into a new mozbuild.sphinx module. I also adjusted the
ReST in to be warning free. It renders oh so nicely

There are a few dozen formatting errors for docstrings in the Python
modules. I hope to one day see that number go down to 0. Should be a
good first bug for somebody.
Attachment #813340 - Flags: review?(mshal)
Assignee: nobody → gps
Comment on attachment 813340 [details] [diff] [review]
Integrate symbols and Python API docs into Sphinx

> @CommandProvider
>-class MozbuildFileCommands(object):
>+class MozbuildFileCommands(MachCommandBase):
>     @Command('mozbuild-reference', category='build-dev',
>         description='View reference documentation on mozbuild files.')
>     @CommandArgument('symbol', default=None, nargs='*',
>         help='Symbol to view help on. If not specified, all will be shown.')
>     @CommandArgument('--name-only', '-n', default=False, action='store_true',
>         help='Print symbol names only.')
>     def reference(self, symbol, name_only=False):
>+        # mozbuild.sphinx imports some Sphinx modules, so we need to be sure
>+        # the optional Sphinx package is installed.
>+        self._activate_virtualenv()
>+        self.virtualenv_manager.install_pip_package('Sphinx==1.1.3')
>+        from mozbuild.sphinx import (
>+            format_module,
>+            function_reference,
>+            special_reference,
>+            variable_reference,
>+        )
>+        import mozbuild.frontend.sandbox_symbols as m

Is it good form to import as a single character name? We don't seem to do that anywhere else.

Also, what is the rationale for checking in files that are generated automatically? In general I don't like it since it makes it confusing what is actually edit-able by a developer, and what needs to be reviewed by a reviewer. But since it seems these files are already in the tree, I don't see this as a reason to block this patch.
Attachment #813340 - Flags: review?(mshal) → review+

I don't like auto-generated files in the tree either. But doing this right would require some code to aggregate static files with auto-generated files and I figured it was easier to ignore the problem for now. Followup bug.
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.