[{"id":27630,"web_url":"https://patchwork.libcamera.org/comment/27630/","msgid":"<169055978352.3467343.10063663344215427380@Monstersaurus>","date":"2023-07-28T15:56:23","subject":"Re: [libcamera-devel] [PATCH] utils: checkstyle.py: Extract title\n\tand trailers with one command","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2023-07-25 14:55:47)\n> The Amendment class calls `git show` twice, once to extract the commit\n> title, and a second time to extract the trailers. This can be combined\n> in a single command, which is more efficient. Do so.\n> \n> While at it, centralize initialization of self._trailers in the\n> Commit.__init__() function.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> utils/checkstyle.py | 23 ++++++++++-------------\n> 1 file changed, 10 insertions(+), 13 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index 214509bc74d4..836ea80fe89b 100755\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -206,10 +206,10 @@ class CommitFile:\n> class Commit:\n> def __init__(self, commit):\n> self.commit = commit\n> + self._trailers = []\n> self._parse()\n> \n> def _parse_trailers(self, lines):\n> - self._trailers = []\n> for index in range(1, len(lines)):\n> line = lines[index]\n> if not line:\n> @@ -257,9 +257,6 @@ class StagedChanges(Commit):\n> def __init__(self):\n> Commit.__init__(self, '')\n> \n> - # There are no trailers to parse on a Staged Change.\n> - self._trailers = []\n> -\n> def _parse(self):\n> ret = subprocess.run(['git', 'diff', '--staged', '--name-status'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> @@ -278,21 +275,21 @@ class Amendment(Commit):\n> Commit.__init__(self, '')\n> \n> def _parse(self):\n> - # Create a title using HEAD commit\n> - ret = subprocess.run(['git', 'show', '--pretty=oneline', '--no-patch'],\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> + '--no-patch'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> - self._title = 'Amendment of ' + ret.strip()\n> + lines = ret.splitlines()\n> +\n> + self._title = 'Amendment of ' + lines[0].strip()\n> +\n> + self._parse_trailers(lines)\n\nAha - I'd missed before that _parse_trailers skips the first line\nanyway...\n\n> +\n> # Extract the list of modified files\n> ret = subprocess.run(['git', 'diff', '--staged', '--name-status', 'HEAD~'],\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> self._files = [CommitFile(f) for f in ret.splitlines()]\n> \n> - # Parse trailers from the existing commit only.\n> - ret = subprocess.run(['git', 'show', '--format=%n%(trailers:only,unfold)',\n> - '--no-patch'],\n> - stdout=subprocess.PIPE).stdout.decode('utf-8')\n> - self._parse_trailers(ret.splitlines())\n\nWhich I suspect is where the --format=%n (which seems to be a newline)\nwas fortunate enough to cover my misunderstanding.\n\nAnyway\n\n\nReviewed-by: Kieran Bingham \n\n\n> -\n> def get_diff(self, top_level, filename):\n> diff = subprocess.run(['git', 'diff', '--staged', 'HEAD~', '--',\n> '%s/%s' % (top_level, filename)],\n> \n> base-commit: baaad1bf9e2acb3ab721945041ef46496951c04c\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 BBAB5BDB13\n\tfor ;\n\tFri, 28 Jul 2023 15:56:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D91DE627EB;\n\tFri, 28 Jul 2023 17:56:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 905B7627E6\n\tfor ;\n\tFri, 28 Jul 2023 17:56:26 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F29FC755;\n\tFri, 28 Jul 2023 17:55:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690559788;\n\tbh=aE5fcIC2QVmc2xSPDKKVg21D9MxxhHPFgxD/8CjTO+I=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=P2u2CXNdOVprv9ttZjjXfrLhKNqFDuBaWebGnoM/ROChsSx54x/xrIa1KuQyXTWDI\n\tHXDOOYKS8R0llUbIlGhx7TmlxwJADg8dU0xiNO8T7BPafNA5G6NOFu1ScjCi4y6SjL\n\tuNVgkHuYKboGRxwtEZbzGGn4TLsVkvXgtLzB5EoXhUFOKK+nCmFiq/i0FrjE3r2jZS\n\tkGmLIUyQk69tT0aemSvFD8FAT/RYDQGd3xLrTfiY3afaUTQZqcPrmfgPmRxr9oAPV9\n\tYKlpBS/FfNzOJ1ITsywdC2rIEuRH76oMOqM3FtDox01r9XrqP4Z7lN6Fsnqovf/z8v\n\tMu0MxUI2XYZng==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1690559726;\n\tbh=aE5fcIC2QVmc2xSPDKKVg21D9MxxhHPFgxD/8CjTO+I=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uc1AvjzKReklIBezAmF9comvITWzen5+xrz6O5Yhs3kgvdvQNRg61jXEuXAbU8ZSe\n\tXq4oBsIez109QLMd7x6dIbwSYMBsf8L7V4E4nQ52qe+jRgXoY57m7MR639uN6h2jkL\n\t50uYZp6HNSf3TQLCyHBdLiL3c8HloR0HColQbeeE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uc1AvjzK\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230725135547.12589-1-laurent.pinchart@ideasonboard.com>","References":"<20230725135547.12589-1-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 28 Jul 2023 16:56:23 +0100","Message-ID":"<169055978352.3467343.10063663344215427380@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] utils: checkstyle.py: Extract title\n\tand trailers with one command","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]