[PATCH v5 04/10] scripts/qapi: document the tool that generated the file

Alex Bennée posted 10 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v5 04/10] scripts/qapi: document the tool that generated the file
Posted by Alex Bennée 1 year, 3 months ago
This makes it a little easier for developers to find where things
where being generated.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230523125000.3674739-5-alex.bennee@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" )
 
 
 @contextmanager
-- 
2.39.2


Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
Posted by Stefan Hajnoczi 1 year, 3 months ago
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 <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230523125000.3674739-5-alex.bennee@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 <stefanha@redhat.com>
Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
Posted by Markus Armbruster 1 year, 3 months ago
Alex Bennée <alex.bennee@linaro.org> writes:

> This makes it a little easier for developers to find where things
> where being generated.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230523125000.3674739-5-alex.bennee@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 <armbru@redhat.com>
Re: [PATCH v5 04/10] scripts/qapi: document the tool that generated the file
Posted by Philippe Mathieu-Daudé 1 year, 3 months ago
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 <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230523125000.3674739-5-alex.bennee@linaro.org>
> ---
>   scripts/qapi/gen.py | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>