[libcamera-devel,v3,5/6] checkstyle: Add support for checking style on amendments

Message ID 20200118035448.230530-6-nicolas@ndufresne.ca
State Superseded
Headers show
Series
  • Add the ability to do pre-commit style check
Related show

Commit Message

Nicolas Dufresne Jan. 18, 2020, 3:54 a.m. UTC
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This introduce a new argument "--amend" and a new special type of
commit "Amendment". It will check the style of changes that are in
the index combined with the changes of the last commit. So this is
the changes that would be applied by "git commit --amend" hence the
name of the argument.

This is needed to implement pre-commit hook.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 utils/checkstyle.py | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Jan. 18, 2020, 6:02 p.m. UTC | #1
Hi Nicolas,

Thank you for the patch.

On Fri, Jan 17, 2020 at 10:54:47PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> This introduce a new argument "--amend" and a new special type of

s/introduce/introduces/

> commit "Amendment". It will check the style of changes that are in
> the index combined with the changes of the last commit. So this is
> the changes that would be applied by "git commit --amend" hence the
> name of the argument.
> 
> This is needed to implement pre-commit hook.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  utils/checkstyle.py | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 1cd5476..8591e4e 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -500,6 +500,26 @@ class StagedChanges(Commit):
>                                stdout=subprocess.PIPE).stdout.decode('utf-8')
>  
>  
> +class Amendment(StagedChanges):
> +    def __init__(self):
> +        Commit.__init__(self, None)

You should call the constructor of StagedChanges, not Commit.

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

> +
> +    def get_info(self, top_level):
> +        # Create a title using HEAD commit
> +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],
> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> +        title = 'Amendment of: ' + ret.splitlines()[0]
> +        # Extract the list of modifier files
> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],
> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> +        return title, ret.splitlines()
> +
> +    def get_diff(self, top_level, filename):
> +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
> +                               '%s/%s' % (top_level, filename)],
> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> +
> +
>  def check_file(top_level, commit, filename):
>      # Extract the line numbers touched by the commit.
>      diff = commit.get_diff(top_level, filename)
> @@ -632,6 +652,8 @@ def main(argv):
>                          help='Code formatter. Default to clang-format if not specified.')
>      parser.add_argument('--staged', '-s', action='store_true',
>                          help='Include the changes in the index. Defaults to False')
> +    parser.add_argument('--amend', '-a', action='store_true',
> +                        help='Include changes in the index and the previous patch combined. Defaults to False')
>      parser.add_argument('revision_range', type=str, default=None, nargs='?',
>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')
>      args = parser.parse_args(argv[1:])
> @@ -670,8 +692,10 @@ def main(argv):
>      revlist = []
>      if args.staged:
>          revlist.append(StagedChanges())
> +    if args.amend:
> +        revlist.append(Amendment())
>  
> -    # If not --staged
> +    # If none of --staged or --amend was passed
>      if len(revlist) == 0:
>          # And no revisions was passed, then default to HEAD
>          if not args.revision_range:
Laurent Pinchart Jan. 18, 2020, 6:33 p.m. UTC | #2
On Sat, Jan 18, 2020 at 08:02:20PM +0200, Laurent Pinchart wrote:
> On Fri, Jan 17, 2020 at 10:54:47PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > This introduce a new argument "--amend" and a new special type of
> 
> s/introduce/introduces/
> 
> > commit "Amendment". It will check the style of changes that are in
> > the index combined with the changes of the last commit. So this is
> > the changes that would be applied by "git commit --amend" hence the
> > name of the argument.
> > 
> > This is needed to implement pre-commit hook.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  utils/checkstyle.py | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > index 1cd5476..8591e4e 100755
> > --- a/utils/checkstyle.py
> > +++ b/utils/checkstyle.py
> > @@ -500,6 +500,26 @@ class StagedChanges(Commit):
> >                                stdout=subprocess.PIPE).stdout.decode('utf-8')
> >  
> >  
> > +class Amendment(StagedChanges):
> > +    def __init__(self):
> > +        Commit.__init__(self, None)
> 
> You should call the constructor of StagedChanges, not Commit.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > +
> > +    def get_info(self, top_level):
> > +        # Create a title using HEAD commit
> > +        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],

And here you can use

        ret = subprocess.run(['git', 'show', '--pretty=oneline', '-s'],

to generate a single line.

> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> > +        title = 'Amendment of: ' + ret.splitlines()[0]
> > +        # Extract the list of modifier files

s/modifier/modified/

> > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],
> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> > +        return title, ret.splitlines()
> > +
> > +    def get_diff(self, top_level, filename):
> > +        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
> > +                               '%s/%s' % (top_level, filename)],
> > +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> > +
> > +
> >  def check_file(top_level, commit, filename):
> >      # Extract the line numbers touched by the commit.
> >      diff = commit.get_diff(top_level, filename)
> > @@ -632,6 +652,8 @@ def main(argv):
> >                          help='Code formatter. Default to clang-format if not specified.')
> >      parser.add_argument('--staged', '-s', action='store_true',
> >                          help='Include the changes in the index. Defaults to False')
> > +    parser.add_argument('--amend', '-a', action='store_true',
> > +                        help='Include changes in the index and the previous patch combined. Defaults to False')
> >      parser.add_argument('revision_range', type=str, default=None, nargs='?',
> >                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')
> >      args = parser.parse_args(argv[1:])
> > @@ -670,8 +692,10 @@ def main(argv):
> >      revlist = []
> >      if args.staged:
> >          revlist.append(StagedChanges())
> > +    if args.amend:
> > +        revlist.append(Amendment())
> >  
> > -    # If not --staged
> > +    # If none of --staged or --amend was passed
> >      if len(revlist) == 0:
> >          # And no revisions was passed, then default to HEAD
> >          if not args.revision_range:

Patch

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index 1cd5476..8591e4e 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -500,6 +500,26 @@  class StagedChanges(Commit):
                               stdout=subprocess.PIPE).stdout.decode('utf-8')
 
 
+class Amendment(StagedChanges):
+    def __init__(self):
+        Commit.__init__(self, None)
+
+    def get_info(self, top_level):
+        # Create a title using HEAD commit
+        ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only', 'HEAD'],
+                             stdout=subprocess.PIPE).stdout.decode('utf-8')
+        title = 'Amendment of: ' + ret.splitlines()[0]
+        # Extract the list of modifier files
+        ret = subprocess.run(['git', 'diff', '--staged', '--name-only', 'HEAD~'],
+                             stdout=subprocess.PIPE).stdout.decode('utf-8')
+        return title, ret.splitlines()
+
+    def get_diff(self, top_level, filename):
+        return subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',
+                               '%s/%s' % (top_level, filename)],
+                              stdout=subprocess.PIPE).stdout.decode('utf-8')
+
+
 def check_file(top_level, commit, filename):
     # Extract the line numbers touched by the commit.
     diff = commit.get_diff(top_level, filename)
@@ -632,6 +652,8 @@  def main(argv):
                         help='Code formatter. Default to clang-format if not specified.')
     parser.add_argument('--staged', '-s', action='store_true',
                         help='Include the changes in the index. Defaults to False')
+    parser.add_argument('--amend', '-a', action='store_true',
+                        help='Include changes in the index and the previous patch combined. Defaults to False')
     parser.add_argument('revision_range', type=str, default=None, nargs='?',
                         help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')
     args = parser.parse_args(argv[1:])
@@ -670,8 +692,10 @@  def main(argv):
     revlist = []
     if args.staged:
         revlist.append(StagedChanges())
+    if args.amend:
+        revlist.append(Amendment())
 
-    # If not --staged
+    # If none of --staged or --amend was passed
     if len(revlist) == 0:
         # And no revisions was passed, then default to HEAD
         if not args.revision_range: