[libcamera-devel,v3,3/6] checkstyle: Introduce a Commit class

2670 diff mbox series

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This introduce a Commit class used in the final revlist list. All the
git command are moved into that class. This class will be used to
introduce new type of commit (StagedChanges and Amendment) needed to implement
pre-commit hook support.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 utils/checkstyle.py | 46 +++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

Hi Nicolas,

Thank you for the patch.

On Fri, Jan 17, 2020 at 10:54:45PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> This introduce a Commit class used in the final revlist list. All the

s/introduce/introduces/

> git command are moved into that class. This class will be used to

s/command/commands/

> introduce new type of commit (StagedChanges and Amendment) needed to implement

s/type/types/

Has there been a newly introduced tax on the letter S ? :-)

> pre-commit hook support.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  utils/checkstyle.py | 46 +++++++++++++++++++++++++++++----------------
>  1 file changed, 30 insertions(+), 16 deletions(-)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 4a14309..828605a 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -458,12 +458,33 @@ class StripTrailingSpaceFormatter(Formatter):
>  # Style checking
>  #
>  
> +class Commit:
> +    def __init__(self, commit):
> +        self.commit = commit
> +
> +    def get_info(self, top_level):

The top_level argument isn't used, you can drop it.

> +        # Get the commit title and list of files.
> +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only',
> +                              self.commit],
> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> +        files = ret.splitlines()
> +        # Returning title and files list as a tuple
> +        return files[0], files[1:]
> +
> +    def get_diff(self, top_level, filename):
> +        return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),
> +                               '--', '%s/%s' % (top_level, filename)],
> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> +
> +    def get_file(self, filename):
> +        return subprocess.run(['git', 'show', '%s:%s' % (self.commit, filename)],
> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> +
> +
>  def check_file(top_level, commit, filename):
>      # Extract the line numbers touched by the commit.
> -    diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--',
> -                           '%s/%s' % (top_level, filename)],
> -                          stdout=subprocess.PIPE).stdout
> -    diff = diff.decode('utf-8').splitlines(True)
> +    diff = commit.get_diff(top_level, filename)
> +    diff = diff.splitlines(True)
>      commit_diff = parse_diff(diff)
>  
>      lines = []
> @@ -476,9 +497,7 @@ def check_file(top_level, commit, filename):
>  
>      # Format the file after the commit with all formatters and compute the diff
>      # between the unformatted and formatted contents.
> -    after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)],
> -                           stdout=subprocess.PIPE).stdout
> -    after = after.decode('utf-8')
> +    after = commit.get_file(filename)
>  
>      formatted = after
>      for formatter in Formatter.formatters(filename):
> @@ -522,12 +541,7 @@ def check_file(top_level, commit, filename):
>  
>  
>  def check_style(top_level, commit):
> -    # Get the commit title and list of files.
> -    ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit],
> -                         stdout=subprocess.PIPE)
> -    files = ret.stdout.decode('utf-8').splitlines()
> -    title = files[0]
> -    files = files[1:]
> +    title, files = commit.get_info(top_level)
>  
>      separator = '-' * len(title)
>      print(separator)
> @@ -557,7 +571,7 @@ def check_style(top_level, commit):
>      return issues
>  
>  
> -def extract_revlist(revs):
> +def extract_commits(revs):
>      """Extract a list of commits on which to operate from a revision or revision
>      range.
>      """
> @@ -576,7 +590,7 @@ def extract_revlist(revs):
>          revlist = ret.stdout.decode('utf-8').splitlines()
>          revlist.reverse()
>  
> -    return revlist
> +    return [Commit(x) for x in revlist]
>  
>  
>  def git_top_level():
> @@ -632,7 +646,7 @@ def main(argv):
>      if top_level is None:
>              return 1
>  
> -    revlist = extract_revlist(args.revision_range)
> +    revlist = extract_commits(args.revision_range)

s/revlist/commits/

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  
>      issues = 0
>      for commit in revlist: