From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 D1AA5210ED110 for ; Tue, 21 Aug 2018 07:20:27 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1E684807689D; Tue, 21 Aug 2018 14:20:27 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-71.rdu2.redhat.com [10.10.121.71]) by smtp.corp.redhat.com (Postfix) with ESMTP id 35B7B2157F4A; Tue, 21 Aug 2018 14:20:25 +0000 (UTC) To: shenglei , edk2-devel@lists.01.org Cc: Jaben Carsey , Ruiyu Ni References: <20180821013548.1384-1-shenglei.zhang@intel.com> <20180821013548.1384-7-shenglei.zhang@intel.com> From: Laszlo Ersek Message-ID: <797f7851-a970-e62a-0c9c-4c0118d6c040@redhat.com> Date: Tue, 21 Aug 2018 16:20:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180821013548.1384-7-shenglei.zhang@intel.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 21 Aug 2018 14:20:27 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Tue, 21 Aug 2018 14:20:27 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v2 6/7] ShellPkg: Remove unused PCDs 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: Tue, 21 Aug 2018 14:20:28 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/21/18 03:35, shenglei wrote: > The PCDs below are unused, so they have been removed from inf. > gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize > gEfiShellPkgTokenSpaceGuid.PcdShellMapNameLength > gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize > gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize > gEfiShellPkgTokenSpaceGuid.PcdShellPrintBufferSize > gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize > gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength > > Cc: Jaben Carsey > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: shenglei > Reviewed-by: Ruiyu Ni > --- > ShellPkg/Application/Shell/Shell.inf | 2 -- > ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf | 2 -- > ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf | 3 --- > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf | 1 - > .../UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf | 2 -- > 5 files changed, 10 deletions(-) I thought we agreed that the patches would be split per module, not per package. I was tempted to make the same comment for the previous patch (for SecurityPkg). However, it modified two files (two modules) in total, so I managed to review the changes in one go. That's not the case with this patch. It modifies five modules. It is hard to review, plus the commit message is factually incorrect. It states that all of the listed PCDs are unused across all of ShellPkg, which is not the case. For example, PcdShellLibAutoInitialize is used by UefiShellLib. Laszlo > > diff --git a/ShellPkg/Application/Shell/Shell.inf b/ShellPkg/Application/Shell/Shell.inf > index d89f85bb76..83049844d6 100644 > --- a/ShellPkg/Application/Shell/Shell.inf > +++ b/ShellPkg/Application/Shell/Shell.inf > @@ -102,10 +102,8 @@ > gEfiShellPkgTokenSpaceGuid.PcdShellRequireHiiPlatform ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellSupportFrameworkHii ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellPageBreakDefault ## CONSUMES > - gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellInsertModeDefault ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellScreenLogCount ## CONSUMES > - gEfiShellPkgTokenSpaceGuid.PcdShellMapNameLength ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellPrintBufferSize ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellForceConsole ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellSupplier ## CONSUMES > diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf > index cedb333b28..c35a3087cf 100644 > --- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf > +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf > @@ -69,5 +69,3 @@ > gEfiLoadedImageDevicePathProtocolGuid ## SOMETIMES_CONSUMES > gEfiHiiPackageListProtocolGuid ## CONSUMES > > -[Pcd] > - gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize ## CONSUMES > diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf > index 8fd3bbd5df..2d07b32266 100644 > --- a/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf > +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/DpDynamicCommand.inf > @@ -71,8 +71,5 @@ > gEfiHiiPackageListProtocolGuid ## CONSUMES > gEfiShellDynamicCommandProtocolGuid ## PRODUCES > > -[Pcd] > - gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize ## CONSUMES > - > [DEPEX] > TRUE > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > index e5e007bef3..a795fb92de 100644 > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf > @@ -353,5 +353,4 @@ > gEfiAdapterInfoUndiIpv6SupportGuid ## SOMETIMES_CONSUMES ## GUID > > [Pcd.common] > - gEfiShellPkgTokenSpaceGuid.PcdShellPrintBufferSize ## CONSUMES > gEfiShellPkgTokenSpaceGuid.PcdShellIncludeNtGuids ## CONSUMES > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf > index 3ea51ec082..ec1f87ae19 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf > @@ -119,8 +119,6 @@ > > [Pcd] > gEfiShellPkgTokenSpaceGuid.PcdShellProfileMask ## CONSUMES > - gEfiShellPkgTokenSpaceGuid.PcdShellFileOperationSize ## CONSUMES > - gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## CONSUMES > > [Protocols] > gEfiPciRootBridgeIoProtocolGuid ## SOMETIMES_CONSUMES >