qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 07/24] python: add directory structure README.rst files


From: John Snow
Subject: Re: [PATCH v4 07/24] python: add directory structure README.rst files
Date: Thu, 18 Feb 2021 12:45:52 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

On 2/16/21 9:47 PM, Cleber Rosa wrote:
On Thu, Feb 11, 2021 at 01:58:39PM -0500, John Snow wrote:
Add short readmes to python/, python/qemu/, python/qemu/machine,
python/qemu/qmp, and python/qemu/utils that explain the directory
hierarchy. These readmes are visible when browsing the source on
e.g. gitlab/github and are designed to help new developers/users quickly
make sense of the source tree.

They are not designed for inclusion in a published manual.

Signed-off-by: John Snow <jsnow@redhat.com>
---
  python/README.rst              | 41 ++++++++++++++++++++++++++++++++++
  python/qemu/README.rst         |  8 +++++++
  python/qemu/machine/README.rst |  9 ++++++++
  python/qemu/qmp/README.rst     |  9 ++++++++
  python/qemu/utils/README.rst   |  9 ++++++++
  5 files changed, 76 insertions(+)
  create mode 100644 python/README.rst
  create mode 100644 python/qemu/README.rst
  create mode 100644 python/qemu/machine/README.rst
  create mode 100644 python/qemu/qmp/README.rst
  create mode 100644 python/qemu/utils/README.rst


It's not often I complain about too much documentation, but I honestly
think this will not scale.  I understand that the intention is to help
users browsing through the directory structure it has a huge potential
for bit rot.


Always a nice problem to have too much instead of too little. ;)

The READMEs at the first two levels seem OK, but the ones at the
subpackages level will be a maintainance nightmare.  I would *very
much* move those (subpackage ones) documentation into the Python file
themselves.


I don't think there's much, if anything, to move into those files. There are some details about how the module relates to the rest of the QEMU tree, but I think those details aren't appropriate to have "in" the python package itself -- they won't apply to whatever environment they get installed in.

diff --git a/python/README.rst b/python/README.rst
new file mode 100644
index 00000000000..6a14b99e104
--- /dev/null
+++ b/python/README.rst
@@ -0,0 +1,41 @@
+QEMU Python Tooling
+===================
+
+This directory houses Python tooling used by the QEMU project to build,
+configure, and test QEMU. It is organized by namespace (``qemu``), and
+then by package (``qemu/machine``, ``qemu/qmp``).
+
+``setup.py`` is used by ``pip`` to install this tooling to the current
+environment. ``setup.cfg`` provides the packaging configuration used by
+setup.py in a setuptools specific format. You will generally invoke it
+by doing one of the following:
+
+1. ``pip3 install .`` will install these packages to your current
+   environment. If you are inside a virtual environment, they will
+   install there. If you are not, it will attempt to install to the
+   global environment, which is not recommended.
+
+2. ``pip3 install --user .`` will install these packages to your user's
+   local python packages. If you are inside of a virtual environment,
+   this will fail.
+
+If you amend the ``-e`` argument, pip will install in "editable" mode;
+which installs a version of the package that installs a forwarder
+pointing to these files, such that the package always reflects the
+latest version in your git tree.
+
+See `Installing packages using pip and virtual environments
+<https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
+for more information.
+
+
+Files in this directory
+-----------------------
+
+- ``qemu/`` Python package source directory.
+- ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
+- ``README.rst`` you are here!
+- ``VERSION`` contains the PEP-440 compliant version used to describe
+  this package; it is referenced by ``setup.cfg``.
+- ``setup.cfg`` houses setuptools package configuration.
+- ``setup.py`` is the setuptools installer used by pip; See above.

Top-level doc seems fine to you though? I expect this directory to be reasonably low-traffic in terms of file additions/removals. If/when we start using sphinx to generate documentation for the qemu packages, we can probably link to e.g. readthedocs from here.

Not ready yet, of course.

diff --git a/python/qemu/README.rst b/python/qemu/README.rst
new file mode 100644
index 00000000000..d04943f526c
--- /dev/null
+++ b/python/qemu/README.rst
@@ -0,0 +1,8 @@
+QEMU Python Namespace
+=====================
+
+This directory serves as the root of a `Python PEP 420 implicit
+namespace package <https://www.python.org/dev/peps/pep-0420/>`_.
+
+Each directory below is assumed to be an installable Python package that
+is available under the ``qemu.<package>`` namespace.

This one is short and sweet. Not at risk of rotting. See below for a suggestion that might appease both of us WRT per-subpackage READMEs.

diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
new file mode 100644
index 00000000000..ac2b4fffb42
--- /dev/null
+++ b/python/qemu/machine/README.rst
@@ -0,0 +1,9 @@
+qemu.machine package
+====================
+
+This package provides core utilities used for testing and debugging
+QEMU. It is used by the iotests, vm tests, acceptance tests, and several
+other utilities in the ./scripts directory. It is not a fully-fledged
+SDK and it is subject to change at any time.
+ >> +See the documentation in ``__init__.py`` for more information.

This is actually *pretty* brief, and I didn't intend for it to be exhaustively complete. I am trying to say "Please look at the python docstrings in __init__.py for real details!" because I was also worried about bitrot and duplicating docs.

Though, sure, it does duplicate at least some of the basic information. We have three-ish choices:

(1) Eh, fine, leave it here.
(2) Update it to be a simple pointer to __init__.py only; i.e. just the title heading and the "Please see __init__.py for details" hint. (3) Just remove it. It should be common knowledge to investigate __init__.py for package-level documentation.
  (3b) Remove it, but update the PEP420-level document to contain an
       extra blurb that says "See each subpackage's __init__.py for more
       information."

Your preference?

diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst
new file mode 100644
index 00000000000..c21951491cf
--- /dev/null
+++ b/python/qemu/qmp/README.rst
@@ -0,0 +1,9 @@
+qemu.qmp package
+================
+
+This package provides a library used for connecting to and communicating
+with QMP servers. It is used extensively by iotests, vm tests,
+acceptance tests, and other utilities in the ./scripts directory. It is
+not a fully-fledged SDK and is subject to change at any time.
+
+See the documentation in ``__init__.py`` for more information.

Same story as above. This is a pretty brief explanation that explains its role in the QEMU source tree, but doesn't explain much about the package itself. The same three options as above make sense to me here.

diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst
new file mode 100644
index 00000000000..4b33c1f27e1
--- /dev/null
+++ b/python/qemu/utils/README.rst
@@ -0,0 +1,9 @@
+qemu.utils package
+==================
+
+This package provides misc utilities used for testing and debugging
+QEMU. It is used most directly by the qemu.machine package, but has some
+uses by the vm and acceptance tests for determining accelerator support.
+
+See the documentation in ``__init__.py`` and ``accel.py`` for more
+information.


I broke my own convention here and mentioned something beside __init__.py. I will remove that.

And example of the bit rot and the huge maintainance cost is when a
new file is added here, let's say, "qemu/utils/network.py".  I think
your good intentions would quickly backfire.


As you point out, that little slip-up of mine gave you room to attack the bitrot :) I avoided it in two other places, but the utils package is sooooo tiny I gave in to trying to be helpful and pointed out the real implementation file.

Regards,
- Cleber.


I'm still somewhat on the fence. I was trying to make the python directories accessible to outside contributors as much as was humanly possible, but the bitrot concern is a good point. I think it can be alleviated by making clear that these files are just little "see also" pointers that should quite likely not rot very quickly.

Lemme know your final thoughts here and I'll adjust accordingly.

--js




reply via email to

[Prev in Thread] Current Thread [Next in Thread]