Re: [PATCH 1/1] ArmVirtPkg: Remove meaningless comment


Philippe Mathieu-Daudé <philmd@...>
 

On 7/6/21 5:57 PM, Laszlo Ersek wrote:
On 07/06/21 11:49, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daude <philmd@...>

The "Shell Embedded Boot Loader" description (added in
commit 6f5872b1f401) does not add any value, remove it.

Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Leif Lindholm <leif@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Julien Grall <julien@...>
Suggested-by: Laszlo Ersek <lersek@...>
Signed-off-by: Philippe Mathieu-Daude <philmd@...>
---
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.
Yes I agree.

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

Join devel@edk2.groups.io to automatically receive all group messages.