[libcamera-devel,4/4] utils: checkstyle.py: Check trailers for Amendment commits
diff mbox series

Message ID 20230711133915.650485-5-kieran.bingham@ideasonboard.com
State Accepted
Commit baaad1bf9e2acb3ab721945041ef46496951c04c
Headers show
Series
  • utils: checkstyle.py: Fix Trailer handling
Related show

Commit Message

Kieran Bingham July 11, 2023, 1:39 p.m. UTC
The commit trailers are checked as part of processing the commit message
with the newly introduced TrailersChecker.

This relies on the trailers property being correctly exposed by the
Commit object, and is implemented for the base Commit but not processed
for Amendment commits.

Refactor the trailer property handling to a helper function in the base
Commit class and make use of it with a newly added call to obtain the
existing Trailers from the most recent commit when using Amendment.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 utils/checkstyle.py | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

Comments

Umang Jain July 11, 2023, 2:23 p.m. UTC | #1
Hi Kieran,


On 7/11/23 7:09 PM, Kieran Bingham via libcamera-devel wrote:
> The commit trailers are checked as part of processing the commit message
> with the newly introduced TrailersChecker.
>
> This relies on the trailers property being correctly exposed by the
> Commit object, and is implemented for the base Commit but not processed
> for Amendment commits.
>
> Refactor the trailer property handling to a helper function in the base
> Commit class and make use of it with a newly added call to obtain the
> existing Trailers from the most recent commit when using Amendment.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   utils/checkstyle.py | 26 ++++++++++++++++++--------
>   1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 5663af811961..214509bc74d4 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -208,6 +208,17 @@ class Commit:
>           self.commit = commit
>           self._parse()
>   
> +    def _parse_trailers(self, lines):
> +        self._trailers = []
> +        for index in range(1, len(lines)):
> +            line = lines[index]
> +            if not line:
> +                break
> +
> +            self._trailers.append(line)
> +
> +        return index
> +
>       def _parse(self):
>           # Get the commit title and list of files.
>           ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',
> @@ -217,14 +228,7 @@ class Commit:
>   
>           self._title = lines[0]
>   
> -        self._trailers = []
> -        for index in range(1, len(lines)):
> -            line = lines[index]
> -            if not line:
> -                break
> -
> -            self._trailers.append(line)
> -
> +        index = self._parse_trailers(lines)
>           self._files = [CommitFile(f) for f in lines[index:] if f]
>   
>       def files(self, filter='AMR'):
> @@ -283,6 +287,12 @@ class Amendment(Commit):
>                                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())
> +
>       def get_diff(self, top_level, filename):
>           diff = subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
>                                  '%s/%s' % (top_level, filename)],

Patch
diff mbox series

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index 5663af811961..214509bc74d4 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -208,6 +208,17 @@  class Commit:
         self.commit = commit
         self._parse()
 
+    def _parse_trailers(self, lines):
+        self._trailers = []
+        for index in range(1, len(lines)):
+            line = lines[index]
+            if not line:
+                break
+
+            self._trailers.append(line)
+
+        return index
+
     def _parse(self):
         # Get the commit title and list of files.
         ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',
@@ -217,14 +228,7 @@  class Commit:
 
         self._title = lines[0]
 
-        self._trailers = []
-        for index in range(1, len(lines)):
-            line = lines[index]
-            if not line:
-                break
-
-            self._trailers.append(line)
-
+        index = self._parse_trailers(lines)
         self._files = [CommitFile(f) for f in lines[index:] if f]
 
     def files(self, filter='AMR'):
@@ -283,6 +287,12 @@  class Amendment(Commit):
                              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())
+
     def get_diff(self, top_level, filename):
         diff = subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
                                '%s/%s' % (top_level, filename)],