[Top][All Lists]

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

Re: [PATCH v6 15/19] qapi/introspect.py: Add docstrings to _gen_tree and

From: John Snow
Subject: Re: [PATCH v6 15/19] qapi/introspect.py: Add docstrings to _gen_tree and _tree_to_qlit
Date: Wed, 17 Feb 2021 11:07:18 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

On 2/17/21 4:39 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

Signed-off-by: John Snow <jsnow@redhat.com>
  scripts/qapi/introspect.py | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 45284af1330..5d4f5e23f7e 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -99,6 +99,15 @@ def __init__(self, value: _ValueT, ifcond: Iterable[str],
  def _tree_to_qlit(obj: JSONValue,
                    level: int = 0,
                    dict_value: bool = False) -> str:
+    """
+    Convert the type tree into a QLIT C string, recursively.
+    :param obj: The value to convert.
+                This value may not be Annotated when dict_value is True.
+    :param level: The indentation level for this particular value.
+    :param dict_value: True when the value being processed belongs to a
+                       dict key; which suppresses the output indent.
+    """
def indent(level: int) -> str:
          return level * 4 * ' '
@@ -244,6 +253,15 @@ def _gen_features(features: List[QAPISchemaFeature]
      def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
                    ifcond: Sequence[str],
                    features: Optional[List[QAPISchemaFeature]]) -> None:
+        """
+        Build and append a SchemaInfo object to self._trees.
+        :param name: The entity's name.
+        :param mtype: The entity's meta-type.
+        :param obj: Additional entity fields, as appropriate for the meta-type.

"Additional members", since we're talking about a JSON object.

I thought "field" was also appropriate for JSON, but I suppose the spec doesn't use that word. Over time, "field", "member" and "property" have become just meaningless word-slurry to me.


"Additional members as appropriate for the meta-type."

+        :param ifcond: Sequence of conditionals that apply to this entity.
+        :param features: Optional features field for SchemaInfo.


"Optional features member for SchemaInfo" ?


Sure we want to restate parts of the type ("Sequence of", "Optional") in
the doc string?

I usually avoid it, but sometimes for non-scalars I found that it read better to give a nod to the plural, as in:

[ifcond is a] sequence of conditionals ...

but, yes, I haven't been consistent about it. right below for @obj I omit the type of the container.

"Conditionals that apply to this entity" feels almost too terse in isolation.

I don't feel like it's a requisite to state the type, but in some cases I unconsciously chose to mention the structure.

With regards to "Optional", I use this word specifically to indicate parameters that have default values -- distinct from a type that's Optional[], which is really actually like Nullable[T] ... If it makes you feel better, Guido says he regrets that naming decision. Oops!

I'm probably not consistent about when I decided to write it, though.

Ehm. If it's not harmful to leave it as-is, I think it'd be okay to do so. If you prefer a consistency all one way or all the other, I'd have to run the vacuum back over the series to check for it.

+        """
          comment: Optional[str] = None
          if mtype not in ('command', 'event', 'builtin', 'array'):
              if not self._unmask:

Also: more line-wrapping for PEP 8.

I thought the 72 column limit was for things like comments and docstrings.

reply via email to

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