comments below

On 07/08/15 08:24, jiaxinwu wrote:
> Version2 update with a proper commit message.
> 
> Cc: Feng Tian <feng.t...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: jiaxinwu <jiaxin...@intel.com>
> ---
>  MdeModulePkg/Include/Guid/Ip4ConfigHii.h           |  25 -
>  MdeModulePkg/Include/Guid/NicIp4ConfigNvData.h     |  70 --
>  MdeModulePkg/MdeModulePkg.dec                      |   6 -
>  MdeModulePkg/MdeModulePkg.dsc                      |   1 -
>  .../Universal/Network/Ip4ConfigDxe/ComponentName.c | 165 ----
>  .../Universal/Network/Ip4ConfigDxe/Ip4Config.c     | 745 -----------------
>  .../Universal/Network/Ip4ConfigDxe/Ip4Config.h     | 533 ------------
>  .../Network/Ip4ConfigDxe/Ip4ConfigDriver.c         | 505 ------------
>  .../Network/Ip4ConfigDxe/Ip4ConfigDxe.inf          |  91 ---
>  .../Network/Ip4ConfigDxe/Ip4ConfigDxe.uni          | Bin 2700 -> 0 bytes
>  .../Network/Ip4ConfigDxe/Ip4ConfigDxe.vfr          |  89 --
>  .../Network/Ip4ConfigDxe/Ip4ConfigDxeExtra.uni     | Bin 1366 -> 0 bytes
>  .../Network/Ip4ConfigDxe/Ip4ConfigDxeStrings.uni   | Bin 3000 -> 0 bytes
>  .../Universal/Network/Ip4ConfigDxe/Ip4ConfigNv.c   | 909 
> ---------------------
>  .../Universal/Network/Ip4ConfigDxe/Ip4ConfigNv.h   |  54 --
>  .../Universal/Network/Ip4ConfigDxe/Ip4NvData.h     |  48 --
>  .../Network/Ip4ConfigDxe/NicIp4Variable.c          | 319 --------
>  .../Network/Ip4ConfigDxe/NicIp4Variable.h          | 104 ---
>  18 files changed, 3664 deletions(-)
>  delete mode 100644 MdeModulePkg/Include/Guid/Ip4ConfigHii.h
>  delete mode 100644 MdeModulePkg/Include/Guid/NicIp4ConfigNvData.h
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/ComponentName.c
>  delete mode 100644 MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4Config.c
>  delete mode 100644 MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4Config.h
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDriver.c
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.uni
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.vfr
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxeExtra.uni
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxeStrings.uni
>  delete mode 100644 MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigNv.c
>  delete mode 100644 MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigNv.h
>  delete mode 100644 MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4NvData.h
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/NicIp4Variable.c
>  delete mode 100644 
> MdeModulePkg/Universal/Network/Ip4ConfigDxe/NicIp4Variable.h

Okay, so this patch has again the exact same two problems:
- bad commit message
- ordering error

First, the commit message should justify why removing the driver *at
this exact point* is safe. The reason should be given as:

    no platform DSC in the edk2 tree references Ip4ConfigDxe.inf
    any longer

Which immediately brings us to the second point, the ordering error.
Assume that the tree is checked out right at this patch. (Which is a
*perfectly* valid assumption.) If someone tried to build OvmfPkg,
ArmVirtPkg, or other packages at that point, the build would fail.
Because, at this point in the series, you remove the driver, but leave
the references in place.

Therefore this patch must be the *last* one in the series, ie. after you
removed the references in all the DSC and FDF files.

Thanks
Laszlo

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to