[libcamera-devel] utils: checkstyle.py: Extract title and trailers with one command

18892 diff mbox series

The Amendment class calls `git show` twice, once to extract the commit
title, and a second time to extract the trailers. This can be combined
in a single command, which is more efficient. Do so.

While at it, centralize initialization of self._trailers in the
Commit.__init__() function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/checkstyle.py | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)


base-commit: baaad1bf9e2acb3ab721945041ef46496951c04c

Quoting Laurent Pinchart (2023-07-25 14:55:47)
> The Amendment class calls `git show` twice, once to extract the commit
> title, and a second time to extract the trailers. This can be combined
> in a single command, which is more efficient. Do so.
> 
> While at it, centralize initialization of self._trailers in the
> Commit.__init__() function.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  utils/checkstyle.py | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 214509bc74d4..836ea80fe89b 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -206,10 +206,10 @@ class CommitFile:
>  class Commit:
>      def __init__(self, commit):
>          self.commit = commit
> +        self._trailers = []
>          self._parse()
>  
>      def _parse_trailers(self, lines):
> -        self._trailers = []
>          for index in range(1, len(lines)):
>              line = lines[index]
>              if not line:
> @@ -257,9 +257,6 @@ class StagedChanges(Commit):
>      def __init__(self):
>          Commit.__init__(self, '')
>  
> -        # There are no trailers to parse on a Staged Change.
> -        self._trailers = []
> -
>      def _parse(self):
>          ret = subprocess.run(['git', 'diff', '--staged', '--name-status'],
>                               stdout=subprocess.PIPE).stdout.decode('utf-8')
> @@ -278,21 +275,21 @@ class Amendment(Commit):
>          Commit.__init__(self, '')
>  
>      def _parse(self):
> -        # Create a title using HEAD commit
> -        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'],
> +        # Create a title using HEAD commit and parse the trailers.
> +        ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unfold)',
> +                             '--no-patch'],
>                               stdout=subprocess.PIPE).stdout.decode('utf-8')
> -        self._title = 'Amendment of ' + ret.strip()
> +        lines = ret.splitlines()
> +
> +        self._title = 'Amendment of ' + lines[0].strip()
> +
> +        self._parse_trailers(lines)

Aha - I'd missed before that _parse_trailers skips the first line
anyway...

> +
>          # Extract the list of modified files
>          ret = subprocess.run(['git', 'diff', '--staged', '--name-status', 'HEAD~'],
>                               stdout=subprocess.PIPE).stdout.decode('utf-8')
>          self._files = [CommitFile(f) for f in ret.splitlines()]
>  
> -        # Parse trailers from the existing commit only.
> -        ret = subprocess.run(['git', 'show', '--format=%n%(trailers:only,unfold)',
> -                             '--no-patch'],
> -                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> -        self._parse_trailers(ret.splitlines())

Which I suspect is where the --format=%n (which seems to be a newline)
was fortunate enough to cover my misunderstanding.

Anyway


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> -
>      def get_diff(self, top_level, filename):
>          diff = subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
>                                 '%s/%s' % (top_level, filename)],
> 
> base-commit: baaad1bf9e2acb3ab721945041ef46496951c04c
> -- 
> Regards,
> 
> Laurent Pinchart
>