Re: [edk2-devel] [Patch 3/3] BaseTools/PatchCheck: Disable text conversion in 'git show'

["Jordan Justen" <jordan.l.justen@intel.com>]

On 2019-08-05 15:01:14, Kinney, Michael D wrote:
> Laszlo,
> 
> The context of this change is only to the PatchCheck.py tool.
> and how that tool uses git show.
> 
> I agree with the summary of very flexible capabilities in git
> to help developers review different types of files.  All of 
> those settings that were added to support UNI files in UTF-16
> file format were very valuable when we had to review the
> text changes to those binary files.  We should be using UTF-8
> now, and we can even extend PatchCheck.py to flag an error if
> a UNI file is in UTF-16 format.

Ah. If we are to the point where we want to actively prevent utf-16 in
the tree, then this sounds like a good idea.

> I still prefer we make this change only to PatchCheck.py to
> prevent false positives and print lines of text that can 
> not be found in a developer's working directory.  I prefer
> this one time change to PatchCheck.py instead of having to
> update .gitattributes whenever the git show features are 
> extended to convert more binary files to text files.

I think it's pretty rare for EDK II to add new binary file types, but
I don't feel too strongly on this. I thought adding Laszlo's settings
to .gitattributes might amount to solving two issues with one change.

Since Laszlo acked this patch, I'll go ahead with:

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

> My expectation is that EDK II Maintainers need to review 
> if a binary file is ok or not for a repo.  I would also be
> ok with adding general rules to PatchCheck.py to generate
> an error if a binary file is added/updated in one of the
> text only repos (edk2, edk2-platforms) and not generate
> an error if a binary file is added/updated in a repo that
> supports binaries (edk-non-osi).