From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 950BF211C7F0A for ; Fri, 8 Feb 2019 00:16:19 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DE51A37E74; Fri, 8 Feb 2019 08:16:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-74.rdu2.redhat.com [10.10.120.74]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9830A648B6; Fri, 8 Feb 2019 08:16:15 +0000 (UTC) To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Antoine Coeur , edk2-devel@lists.01.org References: <1335c2cc-7697-96cb-9121-e8b88cbf31fe@redhat.com> From: Laszlo Ersek Message-ID: Date: Fri, 8 Feb 2019 09:16:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 08 Feb 2019 08:16:19 +0000 (UTC) Subject: Re: [PATCH v2] ArmVirtPkg: Fix various typos X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Feb 2019 08:16:20 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 02/07/19 18:48, Philippe Mathieu-Daudé wrote: > Hi Antoine, > > On 2/7/19 6:13 PM, Antoine Coeur wrote: >> Thank you Laszlo. >> >> Do you have any recommendations regarding the maximum size of a patch for smooth reviewing on this mailing list? >> I have about 9000 lines of additional typos corrections in queue at https://github.com/Coeur/edk2/tree/typo, but I'm afraid that big patches will simply be ignored. > > I tagged your "BaseTools: Fix various typos" patch for review, but it is > true than after reading the diff stats "172 files changed, 513 > insertions(+), 518 deletions(-)" I procrastinated and skipped to the > next patch to review... > > The rule of thumb I learned is the limit to the magic number 20, > probably related to our number of fingers/toes :P > > For example you could split the previous patch in: > BaseTools/Source/C/Common > BaseTools/Source/C/VfrCompile > BaseTools/Source/C/* (other) > BaseTools/Source/Python > > And it would be more digest. > >> If I split them in small patches, how many patches can I post every day? Or should I post a hundred patches at the same time and reviewers won't freak out? > > The list shouldn't have limition in how many patches you can send, the > bottleneck here is the human brain and how many patches a reviewer can take. > > Since your patches are related (all fixes typos), what would help is if > you group your patches in series, instead of sending separately. > > Same here, I recommend the 20 limit. For example you can send various > series of up-to 20 patches, like: > - Board X + Y > - Board Z > - Package X > - Module Y > - Arm > - Intel > >> Or could we exceptionally allow pull requests on GitHub for those typos changes, as it doesn't involve actual code-change? That could be an interesting experiment that would prevent cluttering the mailing-list. > > There is a study in progress to use another workflow than mailing list, > but we are not there yet. > Anyway, don't worry about stressing the mailing list ;) Many small patches work a lot better for most reviewers than a few large patches. In this particular case I would suggest (in agreement with Phil) to post one series per Package, and (as a rule of thumb) one patch per module within the series. The ArmVirtPkg patch was on the limit where I could still reasonably review it. What helped was that I applied it locally for review, and then I could display it with "git show --color --word-diff" as well. In some cases, adding "-b", and/or "--word-diff", and/or "-W", and/or "--find-copies-harder", is extremely helpful for review. Note that this doesn't immediately imply that github pull requests would be better. I much prefer giving feedback on the list, quoting the patch for context, and inserting comments wherever appropriate. My personal conviction remains that the mailing list based workflow is the most effective. OTOH I realize the email setup is difficult for many people (and their numbers could even be growing), so I'm not at all opposed to adopting a web-based workflow. As Phil says, we've evaluated github (and some other websites). While there's definitely room for improvement, github looks like the most acceptable solution to me at this point, after the mailing list. Some other tools are still being evaluated though. For now I too would say, "go clutter the mailing list" -- for the list settings as well, many small patches work better than a few huge ones. Just please make sure they are organized in logical series. Also, please CC the maintainers directly (from Maintainers.txt). No need to repost just for this; it's a hint for the future. Thank you for the contribution! Laszlo