Re: [Patch 3/3] BaseTools: Update PatchCheck to report error for EFI_D_*

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

Hi Jordan,

We request to report error since the DEBUG_* is our recommend coding style and EFI_D_ is obsolete.
Looking for DEBUG and EFI_D_ since only look for EFI_D_ may cause some False error be reported. And I agree that user might put EFI_D_ on a separate line, however I scanned our current edk2 code, no such usage. So I'd like to add this support after we meet the real case.

Best Regards,
Zhu Yonghong


-----Original Message-----
From: Justen, Jordan L 
Sent: Tuesday, October 18, 2016 12:58 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>
Subject: Re: [Patch 3/3] BaseTools: Update PatchCheck to report error for EFI_D_*

On 2016-10-17 01:28:38, Yonghong Zhu wrote:
> In EDK2, DEBUG_* is recommended to be used instead of EFI_D_*. For new 
> code, they should use DEBUG_* macro.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=145
>

Remove blank line. I think you meant 143, not 145.

Instead of looking for DEBUG and EFI_D_, I think we should just look for EFI_D_. It is possible that the user might put the EFI_D_ on a separate line. What do you think?

Should we add 'warning' support to the script first, and then flag this as a warning rather than an error? (I don't think this is too important, because getting an error from the script will not currently prevent the patch from being merged.)

-Jordan

> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
> ---
>  BaseTools/Scripts/PatchCheck.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py 
> b/BaseTools/Scripts/PatchCheck.py index 05f8f6e..3ac0769 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -337,10 +337,15 @@ class GitDiffCheck:
>          if self.hunk_filename is not None:
>              lines.append('File: ' + self.hunk_filename)
>          lines.append('Line: ' + line)
>  
>          self.error(*lines)
> +
> +    DEBUG_macro_re = re.compile(r'''^
> +                                    \s*DEBUG\s*\(\(
> +                                ''',
> +                                re.VERBOSE)
>  
>      def check_added_line(self, line):
>          eol = ''
>          for an_eol in self.line_endings:
>              if line.endswith(an_eol):
> @@ -354,10 +359,13 @@ class GitDiffCheck:
>                                    line)
>          if '    ' in line:
>              self.added_line_error('Tab character used', line)
>          if len(stripped) < len(line):
>              self.added_line_error('Trailing whitespace found', line)
> +        if self.DEBUG_macro_re.match(line):
> +            if 'EFI_D_' in line:
> +                self.added_line_error('EFI_D_* format is used, 
> + recommend to use DEBUG_* format', line)
>  
>      split_diff_re = re.compile(r'''
>                                     (?P<cmd>
>                                         ^ diff \s+ --git \s+ a/.+ \s+ b/.+ $
>                                     )
> --
> 2.6.1.windows.1
>