[{"id":3484,"web_url":"https://patchwork.libcamera.org/comment/3484/","msgid":"<20200117215143.GG1074550@oden.dyn.berto.se>","date":"2020-01-17T21:51:43","subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nicolas,\n\nThanks for your work.\n\nOn 2020-01-17 14:17:30 -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> This introduce a Commit class used in the final revlist list. All the\n> git command are moved into that class. This class will be used to\n> introduce new type of commit (index and amendment) needed to implement\n> pre-commit hook support.\n> \n> Signed-off-by: Nicolas Dufresne \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> utils/checkstyle.py | 43 +++++++++++++++++++++++++++++--------------\n> 1 file changed, 29 insertions(+), 14 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index e7375b3..fb865c8 100644\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -458,12 +458,34 @@ class StripTrailingSpaceFormatter(Formatter):\n> # Style checking\n> #\n> \n> +class Commit:\n> + commit = None\n> +\n> + def __init__(self, commit):\n> + self.commit = commit\n> +\n> + def get_info(self, top_level):\n> + # Get the commit title and list of files.\n> + ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only',\n> + self.commit],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> + files = ret.splitlines()\n> + return files[0], files[1:]\n> +\n> + def get_diff(self, top_level, filename):\n> + return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),\n> + '--', '%s/%s' % (top_level, filename)],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> + def get_file(self, filename):\n> + return subprocess.run(['git', 'show', '%s:%s' % (self.commit, filename)],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n> def check_file(top_level, commit, filename):\n> # Extract the line numbers touched by the commit.\n> - diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--',\n> - '%s/%s' % (top_level, filename)],\n> - stdout=subprocess.PIPE).stdout\n> - diff = diff.decode('utf-8').splitlines(True)\n> + diff = commit.get_diff(top_level, filename)\n> + diff = diff.splitlines(True)\n> commit_diff = parse_diff(diff)\n> \n> lines = []\n> @@ -476,9 +498,7 @@ def check_file(top_level, commit, filename):\n> \n> # Format the file after the commit with all formatters and compute the diff\n> # between the unformatted and formatted contents.\n> - after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)],\n> - stdout=subprocess.PIPE).stdout\n> - after = after.decode('utf-8')\n> + after = commit.get_file(filename)\n> \n> formatted = after\n> for formatter in Formatter.formatters(filename):\n> @@ -522,12 +542,7 @@ def check_file(top_level, commit, filename):\n> \n> \n> def check_style(top_level, commit):\n> - # Get the commit title and list of files.\n> - ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit],\n> - stdout=subprocess.PIPE)\n> - files = ret.stdout.decode('utf-8').splitlines()\n> - title = files[0]\n> - files = files[1:]\n> + title, files = commit.get_info(top_level)\n> \n> separator = '-' * len(title)\n> print(separator)\n> @@ -576,7 +591,7 @@ def extract_revlist(revs):\n> revlist = ret.stdout.decode('utf-8').splitlines()\n> revlist.reverse()\n> \n> - return revlist\n> + return [Commit(x) for x in revlist]\n> \n> \n> def git_top_level():\n> -- \n> 2.24.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2895260782\n\tfor ;\n\tFri, 17 Jan 2020 22:51:45 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id h23so27938470ljc.8\n\tfor ;\n\tFri, 17 Jan 2020 13:51:45 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tv29sm1705674ljd.69.2020.01.17.13.51.43\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 17 Jan 2020 13:51:43 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=sqx00PsEcK/fVogTiMeB/N1+9rBxznA4GuIqgopOHPA=;\n\tb=InL1ikMmcusl/ZpZdO/1M3QeBB/SDvmO28LglTKV1dklNky4uq2+XyOJt98khbJB2x\n\tDSH++z9GyUxjt1ItgSl4vxh3eU2WGKRJasacsVzJ6JAk8ytuKaHc1kvhEW6lus2w9bDs\n\tMoN88nNc8f1yuIKxa2JnBbkOf/0cbPVlEYTKrIlWh/x0cxCtYVD9jTA/1O28y4YP7qbZ\n\tHT6THzeELEREHC8RPDX/spMfmavD+Ak27WFQogl3hsBa0Ft6kFDbPsTQzYqwFbjbr8II\n\toqePK21adan8BCNxWi7Q3g2CBn/iQ9JRYtmV2k3d2JeMfkNWTyMJE570zec4+2LZ507B\n\tk+Zw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=sqx00PsEcK/fVogTiMeB/N1+9rBxznA4GuIqgopOHPA=;\n\tb=qOh2BFCdNVsSrBgmEFBuLljel308QViQRJJm+C4L/34TDgJO/bW/z4W0H/kw7piSjV\n\tazYZBN8k/3xWBZ4iY52prGZEJHV5intbt6xJ3zOxoghrx23dv/SFrzIA4JbnwIhvA/pb\n\tGtAAW7SV46NYzI8RHTlDOQ6F1YtP2DAD2lCVLg46mw86Yo/JyScVQg0pRNv79GVOVMDQ\n\tAfzo5J377GCaer+Iu2h8uHQt1Yiy8oNNYk+MdzlppKPu8qvH7EW5Xs2bBjsg12dv6Zkp\n\tSSWdkwd4BAQbayAtKCaAtQJxr6OeG8PSEG+UG5NfhtNOXjLKj2C9xc5Tui0l/BXfhha6\n\tY7xA==","X-Gm-Message-State":"APjAAAWIFfFHJA2HcknH2NOyDnPob4WAvPc6WHQ3UN+lCWMW0xrt0ZGx\n\tfKrsW+y7MtIqL//4MBo9cuOFEw==","X-Google-Smtp-Source":"APXvYqwI8KRq39u3j0hAP2aed9zbHOqs7tuGdI/K1t65E0nPoMPqBvpsjJve6zZxWMNpFW2y8juQlw==","X-Received":"by 2002:a2e:9ec3:: with SMTP id h3mr6853133ljk.159.1579297904482;\n\tFri, 17 Jan 2020 13:51:44 -0800 (PST)","Date":"Fri, 17 Jan 2020 22:51:43 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Message-ID":"<20200117215143.GG1074550@oden.dyn.berto.se>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-4-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200117191733.198897-4-nicolas@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 17 Jan 2020 21:51:45 -0000"}},{"id":3489,"web_url":"https://patchwork.libcamera.org/comment/3489/","msgid":"<20200117221244.GO5711@pendragon.ideasonboard.com>","date":"2020-01-17T22:12:44","subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Fri, Jan 17, 2020 at 02:17:30PM -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> This introduce a Commit class used in the final revlist list. All the\n> git command are moved into that class. This class will be used to\n> introduce new type of commit (index and amendment) needed to implement\n> pre-commit hook support.\n> \n> Signed-off-by: Nicolas Dufresne \n> ---\n> utils/checkstyle.py | 43 +++++++++++++++++++++++++++++--------------\n> 1 file changed, 29 insertions(+), 14 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index e7375b3..fb865c8 100644\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -458,12 +458,34 @@ class StripTrailingSpaceFormatter(Formatter):\n> # Style checking\n> #\n> \n> +class Commit:\n> + commit = None\n\nThis isn't needed. This field would be accessed through Commit.commit,\nwhich isn't equivalent to self.commit below.\n\n> +\n> + def __init__(self, commit):\n> + self.commit = commit\n> +\n> + def get_info(self, top_level):\n> + # Get the commit title and list of files.\n> + ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-only',\n> + self.commit],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> + files = ret.splitlines()\n> + return files[0], files[1:]\n\nHow about\n\n\tret = ret.splitlines()\n\ttitle = ret[0]\n\tfiles = ret[1:]\n\treturn title, files\n\nto show what we're returning ?\n\n> +\n> + def get_diff(self, top_level, filename):\n> + return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),\n> + '--', '%s/%s' % (top_level, filename)],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> + def get_file(self, filename):\n> + return subprocess.run(['git', 'show', '%s:%s' % (self.commit, filename)],\n> + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n> def check_file(top_level, commit, filename):\n> # Extract the line numbers touched by the commit.\n> - diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--',\n> - '%s/%s' % (top_level, filename)],\n> - stdout=subprocess.PIPE).stdout\n> - diff = diff.decode('utf-8').splitlines(True)\n> + diff = commit.get_diff(top_level, filename)\n> + diff = diff.splitlines(True)\n> commit_diff = parse_diff(diff)\n> \n> lines = []\n> @@ -476,9 +498,7 @@ def check_file(top_level, commit, filename):\n> \n> # Format the file after the commit with all formatters and compute the diff\n> # between the unformatted and formatted contents.\n> - after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)],\n> - stdout=subprocess.PIPE).stdout\n> - after = after.decode('utf-8')\n> + after = commit.get_file(filename)\n> \n> formatted = after\n> for formatter in Formatter.formatters(filename):\n> @@ -522,12 +542,7 @@ def check_file(top_level, commit, filename):\n> \n> \n> def check_style(top_level, commit):\n> - # Get the commit title and list of files.\n> - ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit],\n> - stdout=subprocess.PIPE)\n> - files = ret.stdout.decode('utf-8').splitlines()\n> - title = files[0]\n> - files = files[1:]\n> + title, files = commit.get_info(top_level)\n> \n> separator = '-' * len(title)\n> print(separator)\n> @@ -576,7 +591,7 @@ def extract_revlist(revs):\n> revlist = ret.stdout.decode('utf-8').splitlines()\n> revlist.reverse()\n> \n> - return revlist\n> + return [Commit(x) for x in revlist]\n\nAs you're returning commits, should this function be renamed to\nextract_commits ?\n\nWith these issues addressed,\n\nReviewed-by: Laurent Pinchart \n\n> \n> \n> def git_top_level():","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F27ED60782\n\tfor ;\n\tFri, 17 Jan 2020 23:13:14 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5C1D89A1;\n\tFri, 17 Jan 2020 23:13:14 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579299194;\n\tbh=S1QzPxkdCmnJQZJUVAsFO+2695AQ1oIo5tgJ0I1YkHU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kbHdNRllICvsFi3Ow/nCkP9QDuTcKGO3Qn6GjlIPsYHwacLEHjfwilitmjEZYoMU9\n\t9/mrSWpxnd3sdaiePVnGSQS2t0NymFfuWjfLPbVLP4hzMlGz051w58yFWbTyIe43hP\n\t4XItO/s+Av2W+xOvoiMps8i1rW1RIyC/xYQa5I1Y=","Date":"Sat, 18 Jan 2020 00:12:44 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Message-ID":"<20200117221244.GO5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-4-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200117191733.198897-4-nicolas@ndufresne.ca>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 17 Jan 2020 22:13:15 -0000"}},{"id":3493,"web_url":"https://patchwork.libcamera.org/comment/3493/","msgid":"","date":"2020-01-17T22:32:54","subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 18 janvier 2020 à 00:12 +0200, Laurent Pinchart a écrit :\n> Hi Nicolas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 17, 2020 at 02:17:30PM -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne \n> > \n> > This introduce a Commit class used in the final revlist list. All the\n> > git command are moved into that class. This class will be used to\n> > introduce new type of commit (index and amendment) needed to implement\n> > pre-commit hook support.\n> > \n> > Signed-off-by: Nicolas Dufresne \n> > ---\n> > utils/checkstyle.py | 43 +++++++++++++++++++++++++++++--------------\n> > 1 file changed, 29 insertions(+), 14 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index e7375b3..fb865c8 100644\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -458,12 +458,34 @@ class StripTrailingSpaceFormatter(Formatter):\n> > # Style checking\n> > #\n> > \n> > +class Commit:\n> > + commit = None\n> \n> This isn't needed. This field would be accessed through Commit.commit,\n> which isn't equivalent to self.commit below.\n\nOops, you are right. I'm rusty in python.\n\n> \n> > +\n> > + def __init__(self, commit):\n> > + self.commit = commit\n> > +\n> > + def get_info(self, top_level):\n> > + # Get the commit title and list of files.\n> > + ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-\n> > only',\n> > + self.commit],\n> > + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > + files = ret.splitlines()\n> > + return files[0], files[1:]\n> \n> How about\n> \n> \tret = ret.splitlines()\n> \ttitle = ret[0]\n> \tfiles = ret[1:]\n> \treturn title, files\n> \n> to show what we're returning ?\n\nI'd use a comment for that instead (it's interpreted language after all), what\nabout:\n\n # returning title and files list as a tuple\n\n> \n> > +\n> > + def get_diff(self, top_level, filename):\n> > + return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit,\n> > self.commit),\n> > + '--', '%s/%s' % (top_level, filename)],\n> > + stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > + def get_file(self, filename):\n> > + return subprocess.run(['git', 'show', '%s:%s' % (self.commit,\n> > filename)],\n> > + stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +\n> > def check_file(top_level, commit, filename):\n> > # Extract the line numbers touched by the commit.\n> > - diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '\n> > --',\n> > - '%s/%s' % (top_level, filename)],\n> > - stdout=subprocess.PIPE).stdout\n> > - diff = diff.decode('utf-8').splitlines(True)\n> > + diff = commit.get_diff(top_level, filename)\n> > + diff = diff.splitlines(True)\n> > commit_diff = parse_diff(diff)\n> > \n> > lines = []\n> > @@ -476,9 +498,7 @@ def check_file(top_level, commit, filename):\n> > \n> > # Format the file after the commit with all formatters and compute the\n> > diff\n> > # between the unformatted and formatted contents.\n> > - after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)],\n> > - stdout=subprocess.PIPE).stdout\n> > - after = after.decode('utf-8')\n> > + after = commit.get_file(filename)\n> > \n> > formatted = after\n> > for formatter in Formatter.formatters(filename):\n> > @@ -522,12 +542,7 @@ def check_file(top_level, commit, filename):\n> > \n> > \n> > def check_style(top_level, commit):\n> > - # Get the commit title and list of files.\n> > - ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only',\n> > commit],\n> > - stdout=subprocess.PIPE)\n> > - files = ret.stdout.decode('utf-8').splitlines()\n> > - title = files[0]\n> > - files = files[1:]\n> > + title, files = commit.get_info(top_level)\n> > \n> > separator = '-' * len(title)\n> > print(separator)\n> > @@ -576,7 +591,7 @@ def extract_revlist(revs):\n> > revlist = ret.stdout.decode('utf-8').splitlines()\n> > revlist.reverse()\n> > \n> > - return revlist\n> > + return [Commit(x) for x in revlist]\n> \n> As you're returning commits, should this function be renamed to\n> extract_commits ?\n> \n> With these issues addressed,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > \n> > \n> > def git_top_level():","headers":{"Return-Path":"","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AAD6C60782\n\tfor ;\n\tFri, 17 Jan 2020 23:33:02 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 131B4283C3C;\n\tFri, 17 Jan 2020 22:33:01 +0000 (GMT)"],"Message-ID":"","From":"Nicolas Dufresne ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 17:32:54 -0500","In-Reply-To":"<20200117221244.GO5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-4-nicolas@ndufresne.ca>\n\t<20200117221244.GO5711@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 17 Jan 2020 22:33:02 -0000"}},{"id":3495,"web_url":"https://patchwork.libcamera.org/comment/3495/","msgid":"<20200117223439.GS5711@pendragon.ideasonboard.com>","date":"2020-01-17T22:34:39","subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Fri, Jan 17, 2020 at 05:32:54PM -0500, Nicolas Dufresne wrote:\n> Le samedi 18 janvier 2020 à 00:12 +0200, Laurent Pinchart a écrit :\n> > On Fri, Jan 17, 2020 at 02:17:30PM -0500, Nicolas Dufresne wrote:\n> > > From: Nicolas Dufresne \n> > > \n> > > This introduce a Commit class used in the final revlist list. All the\n> > > git command are moved into that class. This class will be used to\n> > > introduce new type of commit (index and amendment) needed to implement\n> > > pre-commit hook support.\n> > > \n> > > Signed-off-by: Nicolas Dufresne \n> > > ---\n> > > utils/checkstyle.py | 43 +++++++++++++++++++++++++++++--------------\n> > > 1 file changed, 29 insertions(+), 14 deletions(-)\n> > > \n> > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > > index e7375b3..fb865c8 100644\n> > > --- a/utils/checkstyle.py\n> > > +++ b/utils/checkstyle.py\n> > > @@ -458,12 +458,34 @@ class StripTrailingSpaceFormatter(Formatter):\n> > > # Style checking\n> > > #\n> > > \n> > > +class Commit:\n> > > + commit = None\n> > \n> > This isn't needed. This field would be accessed through Commit.commit,\n> > which isn't equivalent to self.commit below.\n> \n> Oops, you are right. I'm rusty in python.\n> \n> > > +\n> > > + def __init__(self, commit):\n> > > + self.commit = commit\n> > > +\n> > > + def get_info(self, top_level):\n> > > + # Get the commit title and list of files.\n> > > + ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-\n> > > only',\n> > > + self.commit],\n> > > + stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > + files = ret.splitlines()\n> > > + return files[0], files[1:]\n> > \n> > How about\n> > \n> > \tret = ret.splitlines()\n> > \ttitle = ret[0]\n> > \tfiles = ret[1:]\n> > \treturn title, files\n> > \n> > to show what we're returning ?\n> \n> I'd use a comment for that instead (it's interpreted language after all), what\n> about:\n> \n> # returning title and files list as a tuple\n\nWith s/returning/Returning/ it works for me.\n\n> > > +\n> > > + def get_diff(self, top_level, filename):\n> > > + return subprocess.run(['git', 'diff', '%s~..%s' % (self.commit,\n> > > self.commit),\n> > > + '--', '%s/%s' % (top_level, filename)],\n> > > + stdout=subprocess.PIPE).stdout.decode('utf-\n> > > 8')\n> > > +\n> > > + def get_file(self, filename):\n> > > + return subprocess.run(['git', 'show', '%s:%s' % (self.commit,\n> > > filename)],\n> > > + stdout=subprocess.PIPE).stdout.decode('utf-\n> > > 8')\n> > > +\n> > > +\n> > > def check_file(top_level, commit, filename):\n> > > # Extract the line numbers touched by the commit.\n> > > - diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '\n> > > --',\n> > > - '%s/%s' % (top_level, filename)],\n> > > - stdout=subprocess.PIPE).stdout\n> > > - diff = diff.decode('utf-8').splitlines(True)\n> > > + diff = commit.get_diff(top_level, filename)\n> > > + diff = diff.splitlines(True)\n> > > commit_diff = parse_diff(diff)\n> > > \n> > > lines = []\n> > > @@ -476,9 +498,7 @@ def check_file(top_level, commit, filename):\n> > > \n> > > # Format the file after the commit with all formatters and compute the\n> > > diff\n> > > # between the unformatted and formatted contents.\n> > > - after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)],\n> > > - stdout=subprocess.PIPE).stdout\n> > > - after = after.decode('utf-8')\n> > > + after = commit.get_file(filename)\n> > > \n> > > formatted = after\n> > > for formatter in Formatter.formatters(filename):\n> > > @@ -522,12 +542,7 @@ def check_file(top_level, commit, filename):\n> > > \n> > > \n> > > def check_style(top_level, commit):\n> > > - # Get the commit title and list of files.\n> > > - ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only',\n> > > commit],\n> > > - stdout=subprocess.PIPE)\n> > > - files = ret.stdout.decode('utf-8').splitlines()\n> > > - title = files[0]\n> > > - files = files[1:]\n> > > + title, files = commit.get_info(top_level)\n> > > \n> > > separator = '-' * len(title)\n> > > print(separator)\n> > > @@ -576,7 +591,7 @@ def extract_revlist(revs):\n> > > revlist = ret.stdout.decode('utf-8').splitlines()\n> > > revlist.reverse()\n> > > \n> > > - return revlist\n> > > + return [Commit(x) for x in revlist]\n> > \n> > As you're returning commits, should this function be renamed to\n> > extract_commits ?\n> > \n> > With these issues addressed,\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > > \n> > > \n> > > def git_top_level():","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B04B160792\n\tfor ;\n\tFri, 17 Jan 2020 23:34:55 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 466369A1;\n\tFri, 17 Jan 2020 23:34:55 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579300495;\n\tbh=IIcfn6hJ56YEBPg6eSi2ftoNmbqcx3jk4oXFIEDtY90=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FVXsOphEDWMuzC/fDOnozUWZSolF0XY5ABsQu0hRbiS4lxSvRKd/KoEt6DQt649ki\n\ttVGQV/iCc/9xzN1O7A2bpACpPnlTSiAcGksfLueH9oZ5H73ru2op10j/hw4WMlPoOZ\n\t/bubj6EecugC5LgZiB7OOAIyqFdXJIDYsqLcS/pw=","Date":"Sat, 18 Jan 2020 00:34:39 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200117223439.GS5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-4-nicolas@ndufresne.ca>\n\t<20200117221244.GO5711@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] checkstyle: Introduce a Commit\n\tclass","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 17 Jan 2020 22:34:55 -0000"}}]