Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
On Wed, May 24, 2023 at 02:39:46PM +0100, Alex Bennée wrote:
> This makes it a little easier for developers to find where things
> where being generated.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Alex Bennée
> Message-Id: <20230523125000.3674739-5-alex.ben...@linaro.org>
> ---
> scripts/qapi/gen.py | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 8f8f784f4a..2ea27ef31c 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -13,6 +13,7 @@
>
> from contextlib import contextmanager
> import os
> +import sys
> import re
> from typing import (
> Dict,
> @@ -162,7 +163,7 @@ def __init__(self, fname: str, blurb: str, pydoc: str):
>
> def _top(self) -> str:
> return mcgen('''
> -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/* AUTOMATICALLY GENERATED by %(tool)s DO NOT MODIFY */
>
> /*
> %(blurb)s
> @@ -174,6 +175,7 @@ def _top(self) -> str:
> */
>
> ''',
> + tool=str(os.path.basename(sys.argv[0])),
Why is str() used here? %s already produces a string representation of
whatever value you give it (e.g. '%s' % 123 -> '123').
Reviewed-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
Alex Bennée writes:
> This makes it a little easier for developers to find where things
> where being generated.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Alex Bennée
> Message-Id: <20230523125000.3674739-5-alex.ben...@linaro.org>
> ---
> scripts/qapi/gen.py | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 8f8f784f4a..2ea27ef31c 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -13,6 +13,7 @@
>
> from contextlib import contextmanager
> import os
> +import sys
> import re
> from typing import (
> Dict,
> @@ -162,7 +163,7 @@ def __init__(self, fname: str, blurb: str, pydoc: str):
>
> def _top(self) -> str:
> return mcgen('''
> -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/* AUTOMATICALLY GENERATED by %(tool)s DO NOT MODIFY */
>
> /*
> %(blurb)s
> @@ -174,6 +175,7 @@ def _top(self) -> str:
> */
>
> ''',
> + tool=str(os.path.basename(sys.argv[0])),
> blurb=self._blurb, copyright=self._copyright)
>
> def _bottom(self) -> str:
> @@ -195,7 +197,9 @@ def _bottom(self) -> str:
>
> class QAPIGenTrace(QAPIGen):
> def _top(self) -> str:
> -return super()._top() + '# AUTOMATICALLY GENERATED, DO NOT
> MODIFY\n\n'
> +return super()._top() + (
> +'# AUTOMATICALLY GENERATED by '
> +f"{os.path.basename(sys.argv[0])}, DO NOT MODIFY\n\n" )
Style nitpicks:
* Please don't mix single and double quotes like that.
* You use an f-string to concatenate strings, within a string
concatenation with operator +. Let's to mix two ways to do the same
thing.
* I find the way you break the long line hard to read.
I'd do
return (super()._top()
+ '# AUTOMATICALLY GENERATED by '
+ os.path.basename(sys.argv[0])
+ ', DO NOT MODIFY\n\n')
>
>
> @contextmanager
With that
Reviewed-by: Markus Armbruster
Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
On 24/5/23 15:39, Alex Bennée wrote: This makes it a little easier for developers to find where things where being generated. Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20230523125000.3674739-5-alex.ben...@linaro.org> --- scripts/qapi/gen.py | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
