[libcamera-devel] [PATCH v2 5/6] checkstyle: Add support for checking style on amendments
Nicolas Dufresne
nicolas.dufresne at collabora.com
Fri Jan 17 23:58:33 CET 2020
Le samedi 18 janvier 2020 à 00:25 +0200, Laurent Pinchart a écrit :
> Hi Nicolas,
>
> Thank you for the patch.
>
> On Fri, Jan 17, 2020 at 02:17:32PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne at 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 at collabora.com>
> > ---
> > utils/checkstyle.py | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> > mode change 100644 => 100755 utils/checkstyle.py
> >
> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > old mode 100644
> > new mode 100755
> > index 8e456cd..7c2ce00
> > --- a/utils/checkstyle.py
> > +++ b/utils/checkstyle.py
> > @@ -501,6 +501,26 @@ class Index(Commit):
> > stdout=subprocess.PIPE).stdout.decode('utf-8')
> >
> >
> > +class Amendment(Index):
> > + def __init__(self):
> > + Commit.__init__(self, None)
> > +
> > + def det_info(self, top_level):
>
> s/det_info/get_info/
>
> This code path has never been tested :-)
Arg, uncommited, sorry.
>
> > + # 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)
> > @@ -633,6 +653,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='Includes changes in the index and the previous patch combined. Defaults to False')
>
> s/Includes/Include/
Ack.
>
> > 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:])
> > @@ -671,6 +693,8 @@ def main(argv):
> > revlist = []
> > if args.staged:
> > revlist.append(Index())
> > + if args.amend:
> > + revlist.append(Amendment())
>
> I think you'll find out that it won't work correctly if you specify both
> --staged and --amend, as they duplicate each other to some extent.
> Should we at the very least ignore --staged if --amend is set ? Or use a
> single parameter that would take two different values so that both can't
> be set together ?
>
> Should we also ignore the list of commits of --staged or --amend is set
> ?
Just tested, and there is bugs in there. Of course it's redundant, but that's a
user problem.
./utils/checkstyle.py --staged --amend HEAD~
--------------
Staged changes
--------------
No style issue detected
--------------------------------------------------------------------------------
---------------
Amendment of: f4079ec6365ef6a5c23ff844e9a92aa93e986f01 checkstyle: Add a pre-
commit hook script
--------------------------------------------------------------------------------
---------------
No style issue detected
--------------------------------------------------------------------------------
-----------------
b240df0a1857a653d7417f1bf6035f552d3815e8 checkstyle: Add support for checking
style on amendments
--------------------------------------------------------------------------------
-----------------
No style issue detected
>
> > # If nothing of --staged or --amend was passed, defaults to HEAD
> > if len(revlist) == 0 and not args.revision_range:
More information about the libcamera-devel
mailing list