Re: [PATCH v2 18/38] qapi/events.py: Move comments into docstrings

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:00:41PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/events.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index 00a9513c16..e859fd7a52 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -52,8 +52,10 @@ def gen_event_send_decl(name: str,
>                   proto=build_event_send_proto(name, arg_type, boxed))
>  
>  
> -# Declare and initialize an object 'qapi' using parameters from 
> build_params()
>  def gen_param_var(typ: QAPISchemaObjectType) -> str:
> +    """
> +    Declare and initialize a qapi object, using parameters from 
> `build_params`.

The mention of "object 'qapi'" is gone, and this seems correct
because there's no object called 'qapi' anywhere in this
function.  So:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Comments/questions for follow up patches, because it's beyond the
scope of this series:

- Was the doc string supposed to say "an object 'param'" instead
  of "an object 'qapi'", as that's the name of the variable it
  declares?
- The "using parameters from build_params()" part was confusing to
  me.  I thought it meant gen_param_var() would call build_params().
  I took a while to notice it actually meant "using the C
  function parameters that were previously declared using
  build_params() at build_event_send_proto()".  I don't know
  how we could make it clearer.

> +    """
>      assert not typ.variants
>      ret = mcgen('''
>      %(c_name)s param = {
> -- 
> 2.26.2
> 

-- 
Eduardo