[edk2-devel] [PATCH 1/1] ArmVirtPkg: Remove meaningless comment

Laszlo Ersek lersek at redhat.com
Tue Jul 6 15:57:01 UTC 2021


On 07/06/21 11:49, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daude <philmd at redhat.com>
> 
> The "Shell Embedded Boot Loader" description (added in
> commit 6f5872b1f401) does not add any value, remove it.
> 
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Leif Lindholm <leif at nuviainc.com>
> Cc: Sami Mujawar <sami.mujawar at arm.com>
> Cc: Julien Grall <julien at xen.org>
> Suggested-by: Laszlo Ersek <lersek at redhat.com>
> Signed-off-by: Philippe Mathieu-Daude <philmd at redhat.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc           | 2 +-
>  ArmVirtPkg/ArmVirtKvmTool.fdf        | 2 +-
>  ArmVirtPkg/ArmVirtXen.fdf            | 2 +-
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index d9abadbe708c..e17238e63803 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -376,7 +376,7 @@ [Components.common]
>    MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
>  
>    #
> -  # UEFI application (Shell Embedded Boot Loader)
> +  # UEFI application
>    #
>    ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
>      <PcdsFixedAtBuild>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
> index 076155199905..8ad67233dc90 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.fdf
> +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
> @@ -174,7 +174,7 @@ [FV.FvMain]
>    INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
>  
>    #
> -  # UEFI application (Shell Embedded Boot Loader)
> +  # UEFI application
>    #
>    INF ShellPkg/Application/Shell/Shell.inf
>    INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
> index 8fbbc2313aff..bab4af446cfb 100644
> --- a/ArmVirtPkg/ArmVirtXen.fdf
> +++ b/ArmVirtPkg/ArmVirtXen.fdf
> @@ -178,7 +178,7 @@ [FV.FvMain]
>    INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  
>    #
> -  # UEFI application (Shell Embedded Boot Loader)
> +  # UEFI application
>    #
>    INF ShellPkg/Application/Shell/Shell.inf
>    INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 5b1d10057545..5ecde9233951 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -100,7 +100,7 @@ [FV.FvMain]
>    INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
>  
>    #
> -  # UEFI application (Shell Embedded Boot Loader)
> +  # UEFI application
>    #
>    INF ShellPkg/Application/Shell/Shell.inf
>    INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> 

I'm really sorry Phil, but even the new comment looks just super weird
to me. I've grepped the edk2 codebase for
"ShellPkg/Application/Shell/Shell.inf", and looked at the leading
context (approx. 30 lines) near every match -- and now I actually think
we don't need *any* comments here. I mean we could say "UEFI Shell
Application", but is that really helpful? I have no idea.

I'll let other ArmVirtPkg reviewers comment on this.

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77528): https://edk2.groups.io/g/devel/message/77528
Mute This Topic: https://groups.io/mt/84016887/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list