Re: [PATCH v2 6/7] ShellPkg: Remove unused PCDs

[Laszlo Ersek <lersek@redhat.com>]

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 <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei <shenglei.zhang@intel.com>
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> ---
>  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
>