Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation

[Laszlo Ersek <lersek@redhat.com>]

On 05/04/18 23:47, Carsey, Jaben wrote:
> Thanks.  I don’t push BaseTools, but will await Liming or Yonghong to review and push.

Ah, sure, I didn't mean that you should skip their review; I thought
you'd push the patch after their review (you co-maintain several
packages, so technically you can push). But, if they push the patch for
you, that's fine as well. :)

Thanks!
Laszlo

> 
> Yonghong/Liming,  Can you strip the trailing space before you commit?
> 
> -Jaben
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, May 04, 2018 2:44 PM
>> To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
>> Cc: Gao, Liming <liming.gao@intel.com>; Zhu, Yonghong
>> <yonghong.zhu@intel.com>
>> Subject: Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal
>> translation
>> Importance: High
>>
>> Hi Jaben.
>>
>> On 05/04/18 22:25, Jaben Carsey wrote:
>>> Commit eece4292acc80 changed a variable name, which was tied directly to
>>> a config file entry. This seperates the internal variable names from
>>> the config file entries by having the internal dict accessed through a
>>> translation of key words.
>>>
>>> added a test when this is run straight from command line.
>>>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Jaben Carsey <jaben.carsey@intel.com>
>>> ---
>>>  BaseTools/Source/Python/Ecc/Configuration.py | 122
>> +++++++++++++++++++-
>>>  1 file changed, 119 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/BaseTools/Source/Python/Ecc/Configuration.py
>> b/BaseTools/Source/Python/Ecc/Configuration.py
>>> index b5b583be8c4a..d305182a2666 100644
>>> --- a/BaseTools/Source/Python/Ecc/Configuration.py
>>> +++ b/BaseTools/Source/Python/Ecc/Configuration.py
>>> @@ -1,7 +1,7 @@
>>>  ## @file
>>>  # This file is used to define class Configuration
>>>  #
>>> -# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
>>> +# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
>>>  # This program and the accompanying materials
>>>  # are licensed and made available under the terms and conditions of the
>> BSD License
>>>  # which accompanies this distribution.  The full text of the license may be
>> found at
>>> @@ -20,6 +20,109 @@ from Common.DataType import *
>>>  from Common.String import *
>>>  from Common.LongFilePathSupport import OpenLongFilePath as open
>>>
>>> +_ConfigFileToInternalTranslation = {
>>> +    # not same
>>> +    "ModifierList":"ModifierSet",
>>> +
>>
>> Git complains that the above line contains trailing whitespace.
>>
>> But, that can be removed when you push this patch.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> I've also run the Ecc tool (I didn't try to execute the self-test), and
>> now it seems to work fine.
>>
>> Tested-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Thanks!
>> Laszlo