Re: [PATCH v2] tools/docs/checktransupdate.py: fix all issues reported by pylint

[Jonathan Corbet <corbet@lwn.net>]

Haoyang LIU <tttturtleruss@gmail.com> writes:

> This patch fixes all issues reported by pylint, including:
> 1. Format issue in logging.
> 2. Variable name style issue.

I'm somewhat unconvinced about that second change.  We haven't come up
with a coding style for Python code in the kernel, but I think we do
want it to look at least a bit like kernel code and not just defer all
decisions to tools like pylint.  I'm not really opposed to this change
either, mind you, but the process of getting there worries me a bit.

> Fixes: 63e96ce050e5 ("scripts: fix all issues reported by pylint")
> Signed-off-by: Haoyang LIU <tttturtleruss@gmail.com>
> ---
> V1 -> V2: fix variable name style name and keep the format consistent with other logging format
>
>  tools/docs/checktransupdate.py | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/docs/checktransupdate.py b/tools/docs/checktransupdate.py
> index e894652369a5..cc07cda667fc 100755
> --- a/tools/docs/checktransupdate.py
> +++ b/tools/docs/checktransupdate.py
> @@ -76,11 +76,11 @@ def get_origin_from_trans_smartly(origin_path, t_from_head):
>      (2) Update the translation through commit HASH (TITLE)
>      """
>      # catch flag for 12-bit commit hash
> -    HASH = r'([0-9a-f]{12})'
> +    hash_re = r'([0-9a-f]{12})'
>      # pattern 1: contains "update to commit HASH"
> -    pat_update_to = re.compile(rf'update to commit {HASH}')
> +    pat_update_to = re.compile(rf'update to commit {hash_re}')
>      # pattern 2: contains "Update the translation through commit HASH"
> -    pat_update_translation = re.compile(rf'Update the translation through commit {HASH}')
> +    pat_update_translation = re.compile(rf'Update the translation through commit {hash_re}')
>  
>      origin_commit_hash = None
>      for line in t_from_head["message"]:
> @@ -131,7 +131,7 @@ def check_per_file(file_path):
>      opath = get_origin_path(file_path)
>  
>      if not os.path.isfile(opath):
> -        logging.error("Cannot find the origin path for {file_path}")
> +        logging.error("Cannot find the origin path for %s", file_path)

Why was this change made?  The first time around, you'd simply added the
obviously missing "f", which seems better?

Thanks,

jon