2023-12-01 05:37:02

by Hu Haowen

[permalink] [raw]
Subject: [PATCH v2] scripts/show_delta: reformat code

Correct some lines in irregular coding style to make them look more
harmonious and fit the common coding regulations in Python.

Signed-off-by: Hu Haowen <[email protected]>
---
scripts/show_delta | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/show_delta b/scripts/show_delta
index 291ad65e3089..33446adce74b 100755
--- a/scripts/show_delta
+++ b/scripts/show_delta
@@ -13,7 +13,7 @@ import sys
import string

def usage():
- print ("""usage: show_delta [<options>] <filename>
+ print("""usage: show_delta [<options>] <filename>

This program parses the output from a set of printk message lines which
have time data prefixed because the CONFIG_PRINTK_TIME option is set, or
@@ -46,7 +46,7 @@ def get_time(line):
raise ValueError

# split on closing bracket
- (time_str, rest) = string.split(line[1:],']',1)
+ (time_str, rest) = string.split(line[1:], ']', 1)
time = string.atof(time_str)

#print "time=", time
@@ -81,9 +81,9 @@ def main():
base_str = ""
filein = ""
for arg in sys.argv[1:]:
- if arg=="-b":
- base_str = sys.argv[sys.argv.index("-b")+1]
- elif arg=="-h":
+ if arg == "-b":
+ base_str = sys.argv[sys.argv.index("-b") + 1]
+ elif arg == "-h":
usage()
else:
filein = arg
@@ -92,7 +92,7 @@ def main():
usage()

try:
- lines = open(filein,"r").readlines()
+ lines = open(filein, "r").readlines()
except:
print ("Problem opening file: %s" % filein)
sys.exit(1)
@@ -111,19 +111,19 @@ def main():
(time, rest) = get_time(line)
except:
continue
- if string.find(rest, base_str)==1:
+ if string.find(rest, base_str) == 1:
base_time = time
found = 1
# stop at first match
break
if not found:
- print ('Couldn\'t find line matching base pattern "%s"' % base_str)
+ print('Couldn\'t find line matching base pattern "%s"' % base_str)
sys.exit(1)
else:
base_time = 0.0

for line in lines:
- print (convert_line(line, base_time),)
+ print(convert_line(line, base_time),)

if __name__ == "__main__":
main()
--
2.34.1


2023-12-01 06:55:34

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v2] scripts/show_delta: reformat code

On Fri, Dec 01, 2023 at 01:35:40PM +0800, Hu Haowen wrote:
> Correct some lines in irregular coding style to make them look more
> harmonious and fit the common coding regulations in Python.
>
> Signed-off-by: Hu Haowen <[email protected]>
> ---
> scripts/show_delta | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/show_delta b/scripts/show_delta
> index 291ad65e3089..33446adce74b 100755
> --- a/scripts/show_delta
> +++ b/scripts/show_delta
> @@ -13,7 +13,7 @@ import sys
> import string
>
> def usage():
> - print ("""usage: show_delta [<options>] <filename>
> + print("""usage: show_delta [<options>] <filename>

Hi,

thanks for your patch. What Miguel already noticed for v1 is valid for
v2, too: there are still inconsistencies in the coding style, e.g.
`print (...)` and `print(...)`.

To simplify a consistent coding style for future work on the script,
using an external tool for reformatting (and mentioning it in the commit
message) would be helpful. Miguel suggested Black or Ruff, I think this
is a good idea.

Kind regards,
Nicolas


Attachments:
(No filename) (1.12 kB)
signature.asc (849.00 B)
Download all attachments

2023-12-01 08:03:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] scripts/show_delta: reformat code

On Fri, Dec 01, 2023 at 01:35:40PM +0800, Hu Haowen wrote:
> Correct some lines in irregular coding style to make them look more
> harmonious and fit the common coding regulations in Python.
>
> Signed-off-by: Hu Haowen <[email protected]>
> ---
> scripts/show_delta | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

2023-12-01 14:39:47

by Hu Haowen

[permalink] [raw]
Subject: Re: [PATCH v2] scripts/show_delta: reformat code


On 2023/12/1 14:49, Nicolas Schier wrote:
> On Fri, Dec 01, 2023 at 01:35:40PM +0800, Hu Haowen wrote:
>> Correct some lines in irregular coding style to make them look more
>> harmonious and fit the common coding regulations in Python.
>>
>> Signed-off-by: Hu Haowen <[email protected]>
>> ---
>> scripts/show_delta | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/scripts/show_delta b/scripts/show_delta
>> index 291ad65e3089..33446adce74b 100755
>> --- a/scripts/show_delta
>> +++ b/scripts/show_delta
>> @@ -13,7 +13,7 @@ import sys
>> import string
>>
>> def usage():
>> - print ("""usage: show_delta [<options>] <filename>
>> + print("""usage: show_delta [<options>] <filename>
> Hi,
>
> thanks for your patch. What Miguel already noticed for v1 is valid for
> v2, too: there are still inconsistencies in the coding style, e.g.
> `print (...)` and `print(...)`.
>
> To simplify a consistent coding style for future work on the script,
> using an external tool for reformatting (and mentioning it in the commit
> message) would be helpful. Miguel suggested Black or Ruff, I think this
> is a good idea.


Just got a glimpse on the usage of Black and realized the convenience
it provides and strictness of code style it supplies. It is pretty
feasible for code style analysis series of Python scripts within the
kernel source.

However, here comes the issue that this tool binds itself to its own
bunches of rules how the code should be formatted by default, resulting
in some kind of scenes which do not match what we want when doing kernel
programming, or more exactly this tool may not follow the rules regulated
by the kernel developers or mentioned within kernel documentation,
which means we are obliged to conduct a programming standard for Python
coding within kernel source internally, and then ask Black to review and
reformat the code accordingly. But this programming standard is absent
currently, consequently it should be specified initially from my
perspective. What is your idea on this?

(The Black code style:
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html)

Thanks,
Hu Haowen


>
> Kind regards,
> Nicolas

2023-12-01 15:33:28

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2] scripts/show_delta: reformat code

On Fri, Dec 1, 2023 at 3:49 PM Hu Haowen <[email protected]> wrote:
>
> Just got a glimpse on the usage of Black and realized the convenience
> it provides and strictness of code style it supplies. It is pretty
> feasible for code style analysis series of Python scripts within the
> kernel source.
>
> However, here comes the issue that this tool binds itself to its own
> bunches of rules how the code should be formatted by default, resulting
> in some kind of scenes which do not match what we want when doing kernel
> programming, or more exactly this tool may not follow the rules regulated
> by the kernel developers or mentioned within kernel documentation,
> which means we are obliged to conduct a programming standard for Python
> coding within kernel source internally, and then ask Black to review and
> reformat the code accordingly. But this programming standard is absent
> currently, consequently it should be specified initially from my
> perspective. What is your idea on this?

This is essentially the same problem we have for C.

For C, what I did was try to find an initial "common enough" style and
document the tool in `Documentation/process/clang-format.rst` so that
maintainers could start to use the tool easily if they so wished, at
their own pace. In other words, the benefit was just having the style
around. Then, maybe, after some years, when the tool is good enough
and maintainers are on board, we can start to think about
`clang-format`ing the kernel.

Now, for Python, we have orders of magnitude less code, so perhaps
using the default options of whatever tool is a possibility. In any
case, it would be a matter of exploring the tools, asking for
feedback, documenting the choice made in `Documentation/`, providing
an example patch formatting one of the existing scripts, etc. The main
benefit would be having decided on a particular approach. I would
still avoid sending tree-wide formatting of all scripts until
maintainers of those scripts agree.

I would also recommend taking the chance to also look at linting and
not just formatting, especially given tools like Ruff provide both.
And if you happen to find an actual issue in an existing script thanks
to the linting, then that would be great and allows you to showcase
their usefulness (and maintainers are probably more likely to welcome
series like that vs. just formatting :)

Hope that helps!

Cheers,
Miguel