[PATCH 1/2] utils: checkstyle.py: Add author property to Commit class

[Laurent Pinchart]

On Tue, Aug 06, 2024 at 09:05:04AM +0200, Stefan Klug wrote:
> Hi Laurent,
> 
> Thank you for the patch.
> 
> On Mon, Aug 05, 2024 at 08:48:06PM +0300, Laurent Pinchart wrote:
> > Extend the Commit class with an author property, retrieved from the
> > commit. It will be used to extend checkers.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  utils/checkstyle.py | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > index 7d480bdf4a2f..1eeb3809f7fe 100755
> > --- a/utils/checkstyle.py
> > +++ b/utils/checkstyle.py
> > @@ -216,7 +216,7 @@ class Commit:
> >          self._parse()
> >  
> >      def _parse_trailers(self, lines):
> > -        for index in range(1, len(lines)):
> > +        for index in range(2, len(lines)):
> >              line = lines[index]
> >              if not line:
> >                  break
> > @@ -227,12 +227,13 @@ class Commit:
> >  
> >      def _parse(self):
> >          # Get the commit title and list of files.
> > -        ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',
> > +        ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s%n%(trailers:only,unfold)', '--name-status',
> 
> Nit: This line starts to be quite long...

I'll wrap it.

> >                                self.commit],
> >                               stdout=subprocess.PIPE).stdout.decode('utf-8')
> >          lines = ret.splitlines()
> >  
> > -        self._title = lines[0]
> > +        self._author = lines[0]
> > +        self._title = lines[1]
> >  
> >          index = self._parse_trailers(lines)
> >          self._files = [CommitFile(f) for f in lines[index:] if f]
> > @@ -240,6 +241,10 @@ class Commit:
> >      def files(self, filter='AMR'):
> >          return [f.filename for f in self._files if f.status in filter]
> >  
> > +    @property
> > +    def author(self):
> > +        return self._author
> > +
> >      @property
> >      def title(self):
> >          return self._title
> > @@ -282,12 +287,13 @@ class Amendment(Commit):
> >  
> >      def _parse(self):
> >          # Create a title using HEAD commit and parse the trailers.
> > -        ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unfold)',
> > +        ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s%n%(trailers:only,unfold)',
> >                               '--no-patch'],
> >                               stdout=subprocess.PIPE).stdout.decode('utf-8')
> >          lines = ret.splitlines()
> >  
> > -        self._title = 'Amendment of ' + lines[0].strip()
> > +        self._author = lines[0]
> > +        self._title = 'Amendment of ' + lines[1].strip()
> 
> This raises the question why the title needs stripping but not the
> author.

I asked myself the same question and didn't orignally investigate. Now
that I have, I think I can drop the strip. It was originally added in
commit 17b3c794095e ("checkstyle: Add support for checking style on
amendments"):

        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'],
                             stdout=subprocess.PIPE).stdout.decode('utf-8')
        title = 'Amendment of ' + ret.strip()

There the strip was needed to drop the newline character. Then commit
4694e441c313 ("utils: checkstyle.py: Extract title and trailers with one
command") modified the code to

        ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unf

                             '--no-patch'],
                             stdout=subprocess.PIPE).stdout.decode('utf-8')
        lines = ret.splitlines()

        self._title = 'Amendment of ' + lines[0].strip()

The splitlines() call removes the new line characters, but the strip()
was kept. I'll drop it in v2.

> Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com> 
> 
> >  
> >          self._parse_trailers(lines)
> >  
> > 
> > base-commit: 8af95d6854889dc66746429ccf8888e3a81f6baf

-- 
Regards,

Laurent Pinchart