public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation
       [not found] <cover.1525465447.git.jaben.carsey@intel.com>
@ 2018-05-04 20:25 ` Jaben Carsey
  2018-05-04 21:44   ` Laszlo Ersek
  0 siblings, 1 reply; 5+ messages in thread
From: Jaben Carsey @ 2018-05-04 20:25 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Yonghong Zhu, Laszlo Ersek

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",
+    
+    # same
+    # please keep this in correct alphabetical order.
+    "AutoCorrect":"AutoCorrect",
+    "BinaryExtList":"BinaryExtList",
+    "CFunctionLayoutCheckAll":"CFunctionLayoutCheckAll",
+    "CFunctionLayoutCheckDataDeclaration":"CFunctionLayoutCheckDataDeclaration",
+    "CFunctionLayoutCheckFunctionBody":"CFunctionLayoutCheckFunctionBody",
+    "CFunctionLayoutCheckFunctionName":"CFunctionLayoutCheckFunctionName",
+    "CFunctionLayoutCheckFunctionPrototype":"CFunctionLayoutCheckFunctionPrototype",
+    "CFunctionLayoutCheckNoInitOfVariable":"CFunctionLayoutCheckNoInitOfVariable",
+    "CFunctionLayoutCheckNoStatic":"CFunctionLayoutCheckNoStatic",
+    "CFunctionLayoutCheckOptionalFunctionalModifier":"CFunctionLayoutCheckOptionalFunctionalModifier",
+    "CFunctionLayoutCheckReturnType":"CFunctionLayoutCheckReturnType",
+    "CheckAll":"CheckAll",
+    "Copyright":"Copyright",
+    "DeclarationDataTypeCheckAll":"DeclarationDataTypeCheckAll",
+    "DeclarationDataTypeCheckEFIAPIModifier":"DeclarationDataTypeCheckEFIAPIModifier",
+    "DeclarationDataTypeCheckEnumeratedType":"DeclarationDataTypeCheckEnumeratedType",
+    "DeclarationDataTypeCheckInOutModifier":"DeclarationDataTypeCheckInOutModifier",
+    "DeclarationDataTypeCheckNoUseCType":"DeclarationDataTypeCheckNoUseCType",
+    "DeclarationDataTypeCheckSameStructure":"DeclarationDataTypeCheckSameStructure",
+    "DeclarationDataTypeCheckStructureDeclaration":"DeclarationDataTypeCheckStructureDeclaration",
+    "DeclarationDataTypeCheckUnionType":"DeclarationDataTypeCheckUnionType",
+    "DoxygenCheckAll":"DoxygenCheckAll",
+    "DoxygenCheckCommand":"DoxygenCheckCommand",
+    "DoxygenCheckCommentDescription":"DoxygenCheckCommentDescription",
+    "DoxygenCheckCommentFormat":"DoxygenCheckCommentFormat",
+    "DoxygenCheckFileHeader":"DoxygenCheckFileHeader",
+    "DoxygenCheckFunctionHeader":"DoxygenCheckFunctionHeader",
+    "GeneralCheckAll":"GeneralCheckAll",
+    "GeneralCheckCarriageReturn":"GeneralCheckCarriageReturn",
+    "GeneralCheckFileExistence":"GeneralCheckFileExistence",
+    "GeneralCheckIndentation":"GeneralCheckIndentation",
+    "GeneralCheckIndentationWidth":"GeneralCheckIndentationWidth",
+    "GeneralCheckLine":"GeneralCheckLine",
+    "GeneralCheckLineWidth":"GeneralCheckLineWidth",
+    "GeneralCheckNoProgma":"GeneralCheckNoProgma",
+    "GeneralCheckNoTab":"GeneralCheckNoTab",
+    "GeneralCheckNo_Asm":"GeneralCheckNo_Asm",
+    "GeneralCheckNonAcsii":"GeneralCheckNonAcsii",
+    "GeneralCheckTabWidth":"GeneralCheckTabWidth",
+    "GeneralCheckUni":"GeneralCheckUni",
+    "HeaderCheckAll":"HeaderCheckAll",
+    "HeaderCheckCFileCommentLicenseFormat":"HeaderCheckCFileCommentLicenseFormat",
+    "HeaderCheckCFileCommentReferenceFormat":"HeaderCheckCFileCommentReferenceFormat",
+    "HeaderCheckCFileCommentStartSpacesNum":"HeaderCheckCFileCommentStartSpacesNum",
+    "HeaderCheckFile":"HeaderCheckFile",
+    "HeaderCheckFileCommentEnd":"HeaderCheckFileCommentEnd",
+    "HeaderCheckFunction":"HeaderCheckFunction",
+    "IncludeFileCheckAll":"IncludeFileCheckAll",
+    "IncludeFileCheckData":"IncludeFileCheckData",
+    "IncludeFileCheckIfndefStatement":"IncludeFileCheckIfndefStatement",
+    "IncludeFileCheckSameName":"IncludeFileCheckSameName",
+    "MetaDataFileCheckAll":"MetaDataFileCheckAll",
+    "MetaDataFileCheckBinaryInfInFdf":"MetaDataFileCheckBinaryInfInFdf",
+    "MetaDataFileCheckGenerateFileList":"MetaDataFileCheckGenerateFileList",
+    "MetaDataFileCheckGuidDuplicate":"MetaDataFileCheckGuidDuplicate",
+    "MetaDataFileCheckLibraryDefinedInDec":"MetaDataFileCheckLibraryDefinedInDec",
+    "MetaDataFileCheckLibraryInstance":"MetaDataFileCheckLibraryInstance",
+    "MetaDataFileCheckLibraryInstanceDependent":"MetaDataFileCheckLibraryInstanceDependent",
+    "MetaDataFileCheckLibraryInstanceOrder":"MetaDataFileCheckLibraryInstanceOrder",
+    "MetaDataFileCheckLibraryNoUse":"MetaDataFileCheckLibraryNoUse",
+    "MetaDataFileCheckModuleFileGuidDuplication":"MetaDataFileCheckModuleFileGuidDuplication",
+    "MetaDataFileCheckModuleFileGuidFormat":"MetaDataFileCheckModuleFileGuidFormat",
+    "MetaDataFileCheckModuleFileNoUse":"MetaDataFileCheckModuleFileNoUse",
+    "MetaDataFileCheckModuleFilePcdFormat":"MetaDataFileCheckModuleFilePcdFormat",
+    "MetaDataFileCheckModuleFilePpiFormat":"MetaDataFileCheckModuleFilePpiFormat",
+    "MetaDataFileCheckModuleFileProtocolFormat":"MetaDataFileCheckModuleFileProtocolFormat",
+    "MetaDataFileCheckPathName":"MetaDataFileCheckPathName",
+    "MetaDataFileCheckPathOfGenerateFileList":"MetaDataFileCheckPathOfGenerateFileList",
+    "MetaDataFileCheckPcdDuplicate":"MetaDataFileCheckPcdDuplicate",
+    "MetaDataFileCheckPcdFlash":"MetaDataFileCheckPcdFlash",
+    "MetaDataFileCheckPcdNoUse":"MetaDataFileCheckPcdNoUse",
+    "MetaDataFileCheckPcdType":"MetaDataFileCheckPcdType",
+    "NamingConventionCheckAll":"NamingConventionCheckAll",
+    "NamingConventionCheckDefineStatement":"NamingConventionCheckDefineStatement",
+    "NamingConventionCheckFunctionName":"NamingConventionCheckFunctionName",
+    "NamingConventionCheckIfndefStatement":"NamingConventionCheckIfndefStatement",
+    "NamingConventionCheckPathName":"NamingConventionCheckPathName",
+    "NamingConventionCheckSingleCharacterVariable":"NamingConventionCheckSingleCharacterVariable",
+    "NamingConventionCheckTypedefStatement":"NamingConventionCheckTypedefStatement",
+    "NamingConventionCheckVariableName":"NamingConventionCheckVariableName",
+    "PredicateExpressionCheckAll":"PredicateExpressionCheckAll",
+    "PredicateExpressionCheckBooleanValue":"PredicateExpressionCheckBooleanValue",
+    "PredicateExpressionCheckComparisonNullType":"PredicateExpressionCheckComparisonNullType",
+    "PredicateExpressionCheckNonBooleanOperator":"PredicateExpressionCheckNonBooleanOperator",
+    "ScanOnlyDirList":"ScanOnlyDirList",
+    "SkipDirList":"SkipDirList",
+    "SkipFileList":"SkipFileList",
+    "SmmCommParaCheckAll":"SmmCommParaCheckAll",
+    "SmmCommParaCheckBufferType":"SmmCommParaCheckBufferType",
+    "SpaceCheckAll":"SpaceCheckAll",
+    "SpellingCheckAll":"SpellingCheckAll",
+    "UniCheckAll":"UniCheckAll",
+    "UniCheckHelpInfo":"UniCheckHelpInfo",
+    "UniCheckPCDInfo":"UniCheckPCDInfo",
+    "Version":"Version"
+    }
+
 ## Configuration
 #
 # This class is used to define all items in configuration file
@@ -297,9 +400,10 @@ class Configuration(object):
             Line = CleanString(Line)
             if Line != '':
                 List = GetSplitValueList(Line, TAB_EQUAL_SPLIT)
-                if List[0] not in self.__dict__:
+                if List[0] not in _ConfigFileToInternalTranslation:
                     ErrorMsg = "Invalid configuration option '%s' was found" % List[0]
                     EdkLogger.error("Ecc", EdkLogger.ECC_ERROR, ErrorMsg, File = Filepath, Line = LineNo)
+                assert _ConfigFileToInternalTranslation[List[0]] in self.__dict__
                 if List[0] == 'ModifierList':
                     List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
                 if List[0] == 'MetaDataFileCheckPathOfGenerateFileList' and List[1] == "":
@@ -312,9 +416,21 @@ class Configuration(object):
                     List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
                 if List[0] == 'Copyright':
                     List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
-                self.__dict__[List[0]] = List[1]
+                self.__dict__[_ConfigFileToInternalTranslation[List[0]]] = List[1]
 
     def ShowMe(self):
         print self.Filename
         for Key in self.__dict__.keys():
             print Key, '=', self.__dict__[Key]
+
+#
+# test that our dict and out class still match in contents.
+#
+if __name__ == '__main__':
+    myconfig = Configuration("BaseTools\Source\Python\Ecc\config.ini")
+    for each in myconfig.__dict__:
+        if each == "Filename":
+            continue
+        assert each in _ConfigFileToInternalTranslation.values()
+    for each in _ConfigFileToInternalTranslation.values():
+        assert each in myconfig.__dict__
-- 
2.16.2.windows.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation
  2018-05-04 20:25 ` [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation Jaben Carsey
@ 2018-05-04 21:44   ` Laszlo Ersek
  2018-05-04 21:47     ` Carsey, Jaben
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2018-05-04 21:44 UTC (permalink / raw)
  To: Jaben Carsey, edk2-devel; +Cc: Liming Gao, Yonghong Zhu

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation
  2018-05-04 21:44   ` Laszlo Ersek
@ 2018-05-04 21:47     ` Carsey, Jaben
  2018-05-04 22:42       ` Laszlo Ersek
  0 siblings, 1 reply; 5+ messages in thread
From: Carsey, Jaben @ 2018-05-04 21:47 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong

Thanks.  I don’t push BaseTools, but will await Liming or Yonghong to review and push.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation
  2018-05-04 21:47     ` Carsey, Jaben
@ 2018-05-04 22:42       ` Laszlo Ersek
  2018-05-05  1:21         ` Zhu, Yonghong
  0 siblings, 1 reply; 5+ messages in thread
From: Laszlo Ersek @ 2018-05-04 22:42 UTC (permalink / raw)
  To: Carsey, Jaben, edk2-devel@lists.01.org; +Cc: Gao, Liming, Zhu, Yonghong

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation
  2018-05-04 22:42       ` Laszlo Ersek
@ 2018-05-05  1:21         ` Zhu, Yonghong
  0 siblings, 0 replies; 5+ messages in thread
From: Zhu, Yonghong @ 2018-05-05  1:21 UTC (permalink / raw)
  To: Laszlo Ersek, Carsey, Jaben, edk2-devel@lists.01.org
  Cc: Gao, Liming, Zhu, Yonghong

It looks good to me.  I will strip the trailing space when push this patch.

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Saturday, May 05, 2018 6:43 AM
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

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-05  1:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <cover.1525465447.git.jaben.carsey@intel.com>
2018-05-04 20:25 ` [PATCH v3 1/1] BaseTools: Ecc - add dict for config file to internal translation Jaben Carsey
2018-05-04 21:44   ` Laszlo Ersek
2018-05-04 21:47     ` Carsey, Jaben
2018-05-04 22:42       ` Laszlo Ersek
2018-05-05  1:21         ` Zhu, Yonghong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox