Re: [PATCH 0/5] Move Shell protocol definitions to MdePkg

["Yao, Jiewen" <jiewen.yao@intel.com>]

Of course, it is fine to give a good name in MdePkg. :)

My concern is only about "Compatibility".
This series patch only updated ArmPkg and EmbeddedPkg.
But there might be more close source IP protected packages using the original header file.

That is why I propose a compatible way: We give a good file name in MdePkg. At same time, we can keep the old file name in ShellPkg to include the good name in MdePkg.

I recommend we give a solution to make close source package unchanged after this patch is applied.

Thank you
Yao Jiewen


From: Tim Lewis [mailto:tim.lewis@insyde.com]
Sent: Saturday, October 15, 2016 12:21 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg

I prefer the renamed .h file, even though I have substantial investment in the current infrastructure.

Why? Because engineers don't have time to remember "how does protocol X translate to header file Y" It should be a consistent rule. How many times have I needed to grep the header file name just because I got a build error because the rule wasn't predictable.

1) remove the EFI_ prefix and the _PROTOCOL  suffix (EFI_BLOCK_IO_PROTOCOL -> BLOCK_IO
2) Convert to upper-and-lower case, BLOCK_IO -> Block_Io
3) Remove the _ (Block_Io -> BlockIo)
4)  add a .h BlockIo -> BlockIo.h

Most protocol and PPI header files already follow this rule. The current header files for shell are among the few exceptions, because they don't follow this.

Tim

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Friday, October 14, 2016 6:14 AM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg

Or if you really think we should give a better name.

My recommendation is:

1)      We add content in MdePkg.

2)      We can keep the old protocol file in ShellPkg, but let .h in shellPkg include the .h in MdePkg.

Then we can avoid duplicated code and make it a compatible solution to avoid other module update.

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Friday, October 14, 2016 9:09 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg

Hi
I think the requests is just to *move*.

There is no request to *rename*.

Can we just move to avoid other update?


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Friday, October 14, 2016 5:44 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> Subject: [edk2] [PATCH 0/5] Move Shell protocol definitions to MdePkg
>
> The patches moves Shell spec defined protocol definitions to MdePkg
> and updates all references.
> Content of ShellBase.h is moved to Protocol/Shell.h and ShellBase.h is
> removed.
>
> Ruiyu Ni (5):
>   ShellPkg: Move SHELL_FREE_NON_NULL from ShellBase.h to ShellLib.h
>   MdePkg: Include Shell/ShellDynamicCommand/ShellParameters
> definitions
>   ArmPkg/LinuxLoader: Reference Shell protocols in MdePkg
>   EmbeddedPkg/FdtPlatformDxe: Reference Shell protocols in MdePkg
>   ShellPkg: Remove Shell/ShellDynamicCommand/ShellParameter
> definitions
>
>  ArmPkg/Application/LinuxLoader/LinuxLoader.h       |   4 +-
>  EmbeddedPkg/Drivers/FdtPlatformDxe/FdtPlatform.h   |   4 +-
>  .../EfiShell.h => MdePkg/Include/Protocol/Shell.h  | 134
> +++++++++++++++++-
>  .../Include/Protocol/ShellDynamicCommand.h         |   7 +-
>  .../Include/Protocol/ShellParameters.h             |   4 +-
>  MdePkg/MdePkg.dec                                  |  15 ++
>  ShellPkg/Application/Shell/Shell.h                 |   5 +-
>  ShellPkg/Include/Library/ShellCommandLib.h         |   5 +-
>  ShellPkg/Include/Library/ShellLib.h                |  14 +-
>  ShellPkg/Include/ShellBase.h                       | 157
> ---------------------
>  ShellPkg/Library/UefiDpLib/Dp.h                    |   3 +-
>  ShellPkg/Library/UefiDpLib/UefiDpLib.h             |   7 +-
>  .../UefiHandleParsingLib/UefiHandleParsingLib.h    |   8 +-
>  .../UefiShellBcfgCommandLib.c                      |   5 +-
>  .../UefiShellCEntryLib/UefiShellCEntryLib.c        |   6 +-
>  .../UefiShellCommandLib/UefiShellCommandLib.h      |   7 +-
>  .../Library/UefiShellDebug1CommandsLib/Compress.c  |   7 +-
>  .../UefiShellDebug1CommandsLib.h                   |   7 +-
>  .../UefiShellDriver1CommandsLib.h                  |   7 +-
>  .../UefiShellLevel1CommandsLib.h                   |   7 +-
>  .../UefiShellLevel2CommandsLib.h                   |   7 +-
>  ShellPkg/Library/UefiShellLevel3CommandsLib/Help.c |   2 +-
>  .../UefiShellLevel3CommandsLib.h                   |   7 +-
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c       |   1 -
>  ShellPkg/Library/UefiShellLib/UefiShellLib.h       |   6 +-
>  .../UefiShellNetwork1CommandsLib.h                 |   3 +-
>  .../UefiShellTftpCommandLib.h                      |   3 +-
>  ShellPkg/ShellPkg.dec                              |   3 -
>  ShellPkg/ShellPkg.dsc                              |   2 +
>  29 files changed, 213 insertions(+), 234 deletions(-)  rename
> ShellPkg/Include/Protocol/EfiShell.h =>
> MdePkg/Include/Protocol/Shell.h (92%)  rename
> ShellPkg/Include/Protocol/EfiShellDynamicCommand.h =>
> MdePkg/Include/Protocol/ShellDynamicCommand.h (92%)  rename
> ShellPkg/Include/Protocol/EfiShellParameters.h =>
> MdePkg/Include/Protocol/ShellParameters.h (92%)  delete mode 100644
> ShellPkg/Include/ShellBase.h
>
> --
> 2.9.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel