[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 41/50] qapi: add a 'unit' pragma
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 41/50] qapi: add a 'unit' pragma |
Date: |
Thu, 14 Dec 2017 15:33:30 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> Hi
>
> On Thu, Dec 14, 2017 at 2:54 PM, Markus Armbruster <address@hidden> wrote:
>> Marc-André Lureau <address@hidden> writes:
>>
>>> Add a pragma that allows to tag the following expressions with a unit
>>> name. By default, an expression has no unit name.
>>
>> Please explain the unit name's intended purpose.
>>
>
> It's syccintly explained in the doc.
The commit message should state the patch's purpose. When the patch
adds documentation, and you'd rather not duplicate it in the commit
message, a short summary plus a pointer there may do.
>>> Signed-off-by: Marc-André Lureau <address@hidden>
>>> ---
>>> scripts/qapi.py | 9 ++++++++-
>>> docs/devel/qapi-code-gen.txt | 3 +++
>>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index eb4ffdc06d..1d0defd638 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -279,10 +279,12 @@ class QAPISchemaParser(object):
>>> self.docs = []
>>> self.cur_doc = None
>>> self.accept()
>>> + self.unit = None
>>>
>>> while self.tok is not None:
>>> info = {'file': fname, 'line': self.line,
>>> - 'parent': self.incl_info}
>>> + 'parent': self.incl_info,
>>> + 'unit': self.unit}
>>> if self.tok == '#':
>>> self.reject_expr_doc()
>>> self.cur_doc = self.get_doc(info)
>>> @@ -371,6 +373,11 @@ class QAPISchemaParser(object):
>>> "Pragma name-case-whitelist must be"
>>> " a list of strings")
>>> name_case_whitelist = value
>>> + elif name == 'unit':
>>> + if not isinstance(value, str):
>>> + raise QAPISemError(info,
>>> + "Pragma 'unit' must be string")
>>> + self.unit = value
>>> else:
>>> raise QAPISemError(info, "Unknown pragma '%s'" % name)
>>>
>>> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
>>> index 24fc6f74ee..37a27cd9d7 100644
>>> --- a/docs/devel/qapi-code-gen.txt
>>> +++ b/docs/devel/qapi-code-gen.txt
>>> @@ -326,6 +326,9 @@ violate the rules on permitted return types. Default
>>> is none.
>>> Pragma 'name-case-whitelist' takes a list of names that may violate
>>> rules on use of upper- vs. lower-case letters. Default is none.
>>>
>>> +Pragma 'unit' takes a string value. It will set the unit name for the
>>> +following expressions in the schema. Most code generator can filter
>>> +based on a unit name. Default is none.
>>
>> Do you mean "most code generators"?
>
> The qapi code/doc generators.
So plural is intended. Easy enough to fix.
>>
>> What does "filtering" mean?
>
> To be able to select a subset of expressions based on the unit name.
Let's spell that out.
>>>
>>> === Struct types ===
>>
>> Humor me: put two spaces after a sentence-ending period.
>>
>
> I don't get what you mean.
This sentence ends with a period, which is followed by two spaces. This
sentence is followed by just one space. I prefer two. A few
inconsistencies have crept into this file over time, let's not add more.
All clear now?