John Snow <js...@redhat.com> writes:
> On 10/7/20 4:12 AM, Markus Armbruster wrote:
>> I keep stumbling over things in later patches that turn out to go back
>> to this one.
>> John Snow <js...@redhat.com> writes:
>>
>>> This is a minor re-work of the entrypoint script. It isolates a
>>> generate() method from the actual command-line mechanism.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>>> Reviewed-by: Cleber Rosa <cr...@redhat.com>
>>> Tested-by: Cleber Rosa <cr...@redhat.com>
>>> ---
>>> scripts/qapi-gen.py | 85 +++++++++++++++++++++++++++++++++------------
>>> 1 file changed, 62 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
>>> index 541e8c1f55d..117b396a595 100644
>>> --- a/scripts/qapi-gen.py
>>> +++ b/scripts/qapi-gen.py
>>> @@ -1,30 +1,77 @@
>>> #!/usr/bin/env python3
>>> -# QAPI generator
>>> -#
>>> +
>>> # This work is licensed under the terms of the GNU GPL, version 2 or
>>> later.
>>> # See the COPYING file in the top-level directory.
>>> +"""
>>> +QAPI Generator
>>> +
>>> +This script is the main entry point for generating C code from the QAPI
>>> schema.
>> PEP 8: For flowing long blocks of text with fewer structural
>> restrictions (docstrings or comments), the line length should be limited
>> to 72 characters.
>>
>
> Eugh. OK, but I don't have a good way to check or enforce this,
> admittedly. I have to change my emacs settings to understand this when
> I hit the reflow key. I don't know if the python mode has a
> context-aware reflow length.
>
> ("I don't disagree, but I'm not immediately sure right now how I will
> make sure I, or anyone else, complies with this. Low priority as a
> result?")
Emacs Python mode is close enough by default: fill-paragraph (bound to
M-q) uses variable fill-column, which defaults to 70. If you want the
extra two columns PEP 8 grants you, I can show you how to bump it to 72
just for Python mode.
You can use fill-paragraph for code, too. I don't myself, because I
disagree with its line breaking decisions too often (and so does PEP 8).
A better Python mode would break code lines more neatly, and with the
width defaulting to 79.
>>> +"""
>>> import argparse
>>> import re
>>> import sys
>>> from qapi.commands import gen_commands
>>> +from qapi.error import QAPIError
>>> from qapi.events import gen_events
>>> from qapi.introspect import gen_introspect
>>> -from qapi.schema import QAPIError, QAPISchema
>>> +from qapi.schema import QAPISchema
>>> from qapi.types import gen_types
>>> from qapi.visit import gen_visit
>>>
>>> -def main(argv):
>>> +DEFAULT_OUTPUT_DIR = ''
>>> +DEFAULT_PREFIX = ''
>>> +
>>> +
>>> +def generate(schema_file: str,
>>> + output_dir: str,
>>> + prefix: str,
>>> + unmask: bool = False,
>>> + builtins: bool = False) -> None:
>>> + """
>>> + generate uses a given schema to produce C code in the target directory.
>> PEP 257: The docstring is a phrase ending in a period. It
>> prescribes
>> the function or method's effect as a command ("Do this", "Return that"),
>> not as a description; e.g. don't write "Returns the pathname ...".
>> Suggest
>> Generate C code for the given schema into the target
>> directory.
>>
>
> OK. I don't mind trying to foster a consistent tone. I clearly
> didn't. I will add a note to my style guide todo.
>
> I give you permission to change the voice in any of my docstrings, or
> to adjust the phrasing to be more technically accurate as you see
> fit. You are the primary maintainer of this code, of course, and you
> will know best.
>
> It will be quicker to give you full permission to just change any of
> the docstrings as you see fit than it will be to play review-respin
> ping-pong.
Me rewriting your commits without your consent is putting words in your
mouth, which I don't want to do.
We can still reduce ping-pong: whenever I can, I don't just say "this
needs improvement", I propose improvements. If you disagree, we talk.
Else, if you have to respin, you make a reasonable effort to take them.
Else, the remaining improvements are trivial (because no respin), and
I'll make them in my tree.
>>> +
>>> + :param schema_file: The primary QAPI schema file.
>>> + :param output_dir: The output directory to store generated code.
>>> + :param prefix: Optional C-code prefix for symbol names.
>>> + :param unmask: Expose non-ABI names through introspection?
>>> + :param builtins: Generate code for built-in types?
>>> +
>>> + :raise QAPIError: On failures.
>>> + """
>> [...]
>>
