* [PATCH 1/1] ArmVirtPkg: Remove meaningless comment
@ 2021-07-06 9:49 Philippe Mathieu-Daudé
2021-07-06 15:57 ` [edk2-devel] " Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-06 9:49 UTC (permalink / raw)
To: devel
Cc: Philippe Mathieu-Daude, Laszlo Ersek, Ard Biesheuvel,
Leif Lindholm, Sami Mujawar, Julien Grall
From: Philippe Mathieu-Daude <philmd@redhat.com>
The "Shell Embedded Boot Loader" description (added in
commit 6f5872b1f401) does not add any value, remove it.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Julien Grall <julien@xen.org>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@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
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH 1/1] ArmVirtPkg: Remove meaningless comment
2021-07-06 9:49 [PATCH 1/1] ArmVirtPkg: Remove meaningless comment Philippe Mathieu-Daudé
@ 2021-07-06 15:57 ` Laszlo Ersek
2021-07-07 17:14 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2021-07-06 15:57 UTC (permalink / raw)
To: devel, philmd; +Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar, Julien Grall
On 07/06/21 11:49, Philippe Mathieu-Daudé wrote:
> From: Philippe Mathieu-Daude <philmd@redhat.com>
>
> The "Shell Embedded Boot Loader" description (added in
> commit 6f5872b1f401) does not add any value, remove it.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Julien Grall <julien@xen.org>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daude <philmd@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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH 1/1] ArmVirtPkg: Remove meaningless comment
2021-07-06 15:57 ` [edk2-devel] " Laszlo Ersek
@ 2021-07-07 17:14 ` Philippe Mathieu-Daudé
2021-07-16 16:55 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-07 17:14 UTC (permalink / raw)
To: Laszlo Ersek, devel
Cc: Ard Biesheuvel, Leif Lindholm, Sami Mujawar, Julien Grall
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@redhat.com>
>>
>> The "Shell Embedded Boot Loader" description (added in
>> commit 6f5872b1f401) does not add any value, remove it.
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Julien Grall <julien@xen.org>
>> Suggested-by: Laszlo Ersek <lersek@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daude <philmd@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.
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [edk2-devel] [PATCH 1/1] ArmVirtPkg: Remove meaningless comment
2021-07-07 17:14 ` Philippe Mathieu-Daudé
@ 2021-07-16 16:55 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2021-07-16 16:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Laszlo Ersek, edk2-devel-groups-io, Ard Biesheuvel, Leif Lindholm,
Sami Mujawar, Julien Grall
On Wed, 7 Jul 2021 at 19:14, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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@redhat.com>
> >>
> >> The "Shell Embedded Boot Loader" description (added in
> >> commit 6f5872b1f401) does not add any value, remove it.
> >>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Leif Lindholm <leif@nuviainc.com>
> >> Cc: Sami Mujawar <sami.mujawar@arm.com>
> >> Cc: Julien Grall <julien@xen.org>
> >> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> >> Signed-off-by: Philippe Mathieu-Daude <philmd@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.
>
> Yes I agree.
>
So do I.
> > 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
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-16 16:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-06 9:49 [PATCH 1/1] ArmVirtPkg: Remove meaningless comment Philippe Mathieu-Daudé
2021-07-06 15:57 ` [edk2-devel] " Laszlo Ersek
2021-07-07 17:14 ` Philippe Mathieu-Daudé
2021-07-16 16:55 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox