[{"id":30610,"web_url":"https://patchwork.libcamera.org/comment/30610/","msgid":"<5gadlt5p5gqm6mjpsveo7yeqrumupsozicvfbtmy3llbtt6uuu@l4nccp4ovnpk>","date":"2024-08-06T07:05:04","subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch.\n\nOn Mon, Aug 05, 2024 at 08:48:06PM +0300, Laurent Pinchart wrote:\n> Extend the Commit class with an author property, retrieved from the\n> commit. It will be used to extend checkers.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> utils/checkstyle.py | 16 +++++++++++-----\n> 1 file changed, 11 insertions(+), 5 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index 7d480bdf4a2f..1eeb3809f7fe 100755\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -216,7 +216,7 @@ class Commit:\n> self._parse()\n> \n> def _parse_trailers(self, lines):\n> - for index in range(1, len(lines)):\n> + for index in range(2, len(lines)):\n> line = lines[index]\n> if not line:\n> break\n> @@ -227,12 +227,13 @@ class Commit:\n> \n> def _parse(self):\n> # Get the commit title and list of files.\n> - ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',\n> + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s%n%(trailers:only,unfold)', '--name-status',\n\nNit: This line starts to be quite long...\n\n\n\n> self.commit],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> lines = ret.splitlines()\n> \n> - self._title = lines[0]\n> + self._author = lines[0]\n> + self._title = lines[1]\n> \n> index = self._parse_trailers(lines)\n> self._files = [CommitFile(f) for f in lines[index:] if f]\n> @@ -240,6 +241,10 @@ class Commit:\n> def files(self, filter='AMR'):\n> return [f.filename for f in self._files if f.status in filter]\n> \n> + @property\n> + def author(self):\n> + return self._author\n> +\n> @property\n> def title(self):\n> return self._title\n> @@ -282,12 +287,13 @@ class Amendment(Commit):\n> \n> def _parse(self):\n> # Create a title using HEAD commit and parse the trailers.\n> - ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unfold)',\n> + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s%n%(trailers:only,unfold)',\n> '--no-patch'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> lines = ret.splitlines()\n> \n> - self._title = 'Amendment of ' + lines[0].strip()\n> + self._author = lines[0]\n> + self._title = 'Amendment of ' + lines[1].strip()\n\nThis raises the question why the title needs stripping but not the\nauthor.\n\nReviewed-by: Stefan Klug \n\nCheers,\nStefan\n\n\n> \n> self._parse_trailers(lines)\n> \n> \n> base-commit: 8af95d6854889dc66746429ccf8888e3a81f6baf\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 02741BE173\n\tfor ;\n\tTue, 6 Aug 2024 07:05:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 136966337E;\n\tTue, 6 Aug 2024 09:05:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74C9D61955\n\tfor ;\n\tTue, 6 Aug 2024 09:05:07 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a68:2b6f:8265:aa72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 70D712C5;\n\tTue, 6 Aug 2024 09:04:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Xm2ghe6F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722927855;\n\tbh=r1bf5zv9WhmHPlfhtRsIP8R68R+DFkVbKNb5SBcRRjc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Xm2ghe6FOsC0sXKW/1/wqEqeuePQGFTSjayJU+1O5CXTgLiqa8Xjd9gI9f9h6Wrxy\n\tAIwAUyYXyVOxr3Wsux0j1dUHJzq5WVyCmewFvCK0kVukz6rstlHsxgyJ8Ly8MCw3mN\n\tRslzHi4pXTxi2mGzJnedr3WSyl3u6wQgTZqJXSSc=","Date":"Tue, 6 Aug 2024 09:05:04 +0200","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","Message-ID":"<5gadlt5p5gqm6mjpsveo7yeqrumupsozicvfbtmy3llbtt6uuu@l4nccp4ovnpk>","References":"<20240805174807.10125-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240805174807.10125-1-laurent.pinchart@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30623,"web_url":"https://patchwork.libcamera.org/comment/30623/","msgid":"<20240806090201.GD32045@pendragon.ideasonboard.com>","date":"2024-08-06T09:02:01","subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 06, 2024 at 09:05:04AM +0200, Stefan Klug wrote:\n> Hi Laurent,\n> \n> Thank you for the patch.\n> \n> On Mon, Aug 05, 2024 at 08:48:06PM +0300, Laurent Pinchart wrote:\n> > Extend the Commit class with an author property, retrieved from the\n> > commit. It will be used to extend checkers.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > utils/checkstyle.py | 16 +++++++++++-----\n> > 1 file changed, 11 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index 7d480bdf4a2f..1eeb3809f7fe 100755\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -216,7 +216,7 @@ class Commit:\n> > self._parse()\n> > \n> > def _parse_trailers(self, lines):\n> > - for index in range(1, len(lines)):\n> > + for index in range(2, len(lines)):\n> > line = lines[index]\n> > if not line:\n> > break\n> > @@ -227,12 +227,13 @@ class Commit:\n> > \n> > def _parse(self):\n> > # Get the commit title and list of files.\n> > - ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',\n> > + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s%n%(trailers:only,unfold)', '--name-status',\n> \n> Nit: This line starts to be quite long...\n\nI'll wrap it.\n\n> > self.commit],\n> > stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > lines = ret.splitlines()\n> > \n> > - self._title = lines[0]\n> > + self._author = lines[0]\n> > + self._title = lines[1]\n> > \n> > index = self._parse_trailers(lines)\n> > self._files = [CommitFile(f) for f in lines[index:] if f]\n> > @@ -240,6 +241,10 @@ class Commit:\n> > def files(self, filter='AMR'):\n> > return [f.filename for f in self._files if f.status in filter]\n> > \n> > + @property\n> > + def author(self):\n> > + return self._author\n> > +\n> > @property\n> > def title(self):\n> > return self._title\n> > @@ -282,12 +287,13 @@ class Amendment(Commit):\n> > \n> > def _parse(self):\n> > # Create a title using HEAD commit and parse the trailers.\n> > - ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unfold)',\n> > + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s%n%(trailers:only,unfold)',\n> > '--no-patch'],\n> > stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > lines = ret.splitlines()\n> > \n> > - self._title = 'Amendment of ' + lines[0].strip()\n> > + self._author = lines[0]\n> > + self._title = 'Amendment of ' + lines[1].strip()\n> \n> This raises the question why the title needs stripping but not the\n> author.\n\nI asked myself the same question and didn't orignally investigate. Now\nthat I have, I think I can drop the strip. It was originally added in\ncommit 17b3c794095e (\"checkstyle: Add support for checking style on\namendments\"):\n\n ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'],\n stdout=subprocess.PIPE).stdout.decode('utf-8')\n title = 'Amendment of ' + ret.strip()\n\nThere the strip was needed to drop the newline character. Then commit\n4694e441c313 (\"utils: checkstyle.py: Extract title and trailers with one\ncommand\") modified the code to\n\n ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unf\n\n '--no-patch'],\n stdout=subprocess.PIPE).stdout.decode('utf-8')\n lines = ret.splitlines()\n\n self._title = 'Amendment of ' + lines[0].strip()\n\nThe splitlines() call removes the new line characters, but the strip()\nwas kept. I'll drop it in v2.\n\n> Reviewed-by: Stefan Klug \n> \n> > \n> > self._parse_trailers(lines)\n> > \n> > \n> > base-commit: 8af95d6854889dc66746429ccf8888e3a81f6baf","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 922ABBE173\n\tfor ;\n\tTue, 6 Aug 2024 09:02:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9273663398;\n\tTue, 6 Aug 2024 11:02:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA2816337E\n\tfor ;\n\tTue, 6 Aug 2024 11:02:24 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 987BA2C5;\n\tTue, 6 Aug 2024 11:01:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qaZsw/42\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722934892;\n\tbh=ns8lPeuN+bm0uqq/lW0Z1pJGX9ttT3aBolwP1QB+9EE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qaZsw/428DARWjrx6knnDmq/RK6ee3NygvlUYSuOyeKw8prWLhjKjhPWKMUtej2SP\n\tNNF+pc/WKusc3gRuRlgt0Q0xUw8H0TDXbGdKQtVnvi8U4UZ5FkEueue0tVpsswWs2w\n\teGPyS3ULhUyu3oGt1n27bAT1WHvTQGN+hyuuYnT4=","Date":"Tue, 6 Aug 2024 12:02:01 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","Message-ID":"<20240806090201.GD32045@pendragon.ideasonboard.com>","References":"<20240805174807.10125-1-laurent.pinchart@ideasonboard.com>\n\t<5gadlt5p5gqm6mjpsveo7yeqrumupsozicvfbtmy3llbtt6uuu@l4nccp4ovnpk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<5gadlt5p5gqm6mjpsveo7yeqrumupsozicvfbtmy3llbtt6uuu@l4nccp4ovnpk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30658,"web_url":"https://patchwork.libcamera.org/comment/30658/","msgid":"<172302603065.1687952.6111959304520070335@ping.linuxembedded.co.uk>","date":"2024-08-07T10:20:30","subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-08-06 10:02:01)\n> On Tue, Aug 06, 2024 at 09:05:04AM +0200, Stefan Klug wrote:\n> > Hi Laurent,\n> > \n> > Thank you for the patch.\n> > \n> > On Mon, Aug 05, 2024 at 08:48:06PM +0300, Laurent Pinchart wrote:\n> > > Extend the Commit class with an author property, retrieved from the\n> > > commit. It will be used to extend checkers.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > utils/checkstyle.py | 16 +++++++++++-----\n> > > 1 file changed, 11 insertions(+), 5 deletions(-)\n> > > \n> > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > > index 7d480bdf4a2f..1eeb3809f7fe 100755\n> > > --- a/utils/checkstyle.py\n> > > +++ b/utils/checkstyle.py\n> > > @@ -216,7 +216,7 @@ class Commit:\n> > > self._parse()\n> > > \n> > > def _parse_trailers(self, lines):\n> > > - for index in range(1, len(lines)):\n> > > + for index in range(2, len(lines)):\n> > > line = lines[index]\n> > > if not line:\n> > > break\n> > > @@ -227,12 +227,13 @@ class Commit:\n> > > \n> > > def _parse(self):\n> > > # Get the commit title and list of files.\n> > > - ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',\n> > > + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s%n%(trailers:only,unfold)', '--name-status',\n> > \n> > Nit: This line starts to be quite long...\n> \n> I'll wrap it.\n> \n> > > self.commit],\n> > > stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > lines = ret.splitlines()\n> > > \n> > > - self._title = lines[0]\n> > > + self._author = lines[0]\n> > > + self._title = lines[1]\n> > > \n> > > index = self._parse_trailers(lines)\n> > > self._files = [CommitFile(f) for f in lines[index:] if f]\n> > > @@ -240,6 +241,10 @@ class Commit:\n> > > def files(self, filter='AMR'):\n> > > return [f.filename for f in self._files if f.status in filter]\n> > > \n> > > + @property\n> > > + def author(self):\n> > > + return self._author\n> > > +\n> > > @property\n> > > def title(self):\n> > > return self._title\n> > > @@ -282,12 +287,13 @@ class Amendment(Commit):\n> > > \n> > > def _parse(self):\n> > > # Create a title using HEAD commit and parse the trailers.\n> > > - ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unfold)',\n> > > + ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s%n%(trailers:only,unfold)',\n> > > '--no-patch'],\n> > > stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > lines = ret.splitlines()\n> > > \n> > > - self._title = 'Amendment of ' + lines[0].strip()\n> > > + self._author = lines[0]\n> > > + self._title = 'Amendment of ' + lines[1].strip()\n> > \n> > This raises the question why the title needs stripping but not the\n> > author.\n> \n> I asked myself the same question and didn't orignally investigate. Now\n> that I have, I think I can drop the strip. It was originally added in\n> commit 17b3c794095e (\"checkstyle: Add support for checking style on\n> amendments\"):\n> \n> ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> title = 'Amendment of ' + ret.strip()\n> \n> There the strip was needed to drop the newline character. Then commit\n> 4694e441c313 (\"utils: checkstyle.py: Extract title and trailers with one\n> command\") modified the code to\n> \n> ret = subprocess.run(['git', 'show', '--format=%H %s%n%(trailers:only,unf\n> \n> '--no-patch'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> lines = ret.splitlines()\n> \n> self._title = 'Amendment of ' + lines[0].strip()\n> \n> The splitlines() call removes the new line characters, but the strip()\n> was kept. I'll drop it in v2.\n\nI'm fine with all that.\n\nReviewed-by: Kieran Bingham \n\n> \n> > Reviewed-by: Stefan Klug \n> > \n> > > \n> > > self._parse_trailers(lines)\n> > > \n> > > \n> > > base-commit: 8af95d6854889dc66746429ccf8888e3a81f6baf\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 07FCEC323E\n\tfor ;\n\tWed, 7 Aug 2024 10:20:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1389B6338D;\n\tWed, 7 Aug 2024 12:20:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 13B566337E\n\tfor ;\n\tWed, 7 Aug 2024 12:20:33 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 31C192EC;\n\tWed, 7 Aug 2024 12:19:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"q2XCPdwP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723025980;\n\tbh=pOJtrEikmOmeRNVRLXzuAreiiuj+RWbkyelf7h9pPNA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=q2XCPdwPoaCrnZsIx+94mmkwnERzMV9bj+ELPBqSEXGo8v8dpHR5tIGlOAgYuV28Z\n\tOQeBPwKyMof8rxpTWFrY9Y+l5+iR6ax0PdYHsWWcSy2WI8YbxJmVU1Oq7n/QUCtBkP\n\tJlpE8xGt2aj/MUM48IJ03ir8NpX0DNqfW8EEcqcs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240806090201.GD32045@pendragon.ideasonboard.com>","References":"<20240805174807.10125-1-laurent.pinchart@ideasonboard.com>\n\t<5gadlt5p5gqm6mjpsveo7yeqrumupsozicvfbtmy3llbtt6uuu@l4nccp4ovnpk>\n\t<20240806090201.GD32045@pendragon.ideasonboard.com>","Subject":"Re: [PATCH 1/2] utils: checkstyle.py: Add author property to Commit\n\tclass","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"Laurent Pinchart ,\n\tStefan Klug ","Date":"Wed, 07 Aug 2024 11:20:30 +0100","Message-ID":"<172302603065.1687952.6111959304520070335@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]