From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: jordan.l.justen@intel.com) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by groups.io with SMTP; Wed, 07 Aug 2019 12:42:45 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Aug 2019 12:42:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,358,1559545200"; d="scan'208";a="373885419" Received: from bquerbac-mobl.amr.corp.intel.com (HELO localhost) ([10.252.129.85]) by fmsmga005.fm.intel.com with ESMTP; 07 Aug 2019 12:42:43 -0700 MIME-Version: 1.0 In-Reply-To: References: <20190802001314.25980-1-michael.d.kinney@intel.com> <20190802001314.25980-4-michael.d.kinney@intel.com> <156477054101.20290.17514635538979393303@jljusten-skl> <8d511f89-f2a8-4d29-65e3-cc76fee12017@redhat.com> Subject: Re: [edk2-devel] [Patch 3/3] BaseTools/PatchCheck: Disable text conversion in 'git show' From: "Jordan Justen" To: "Kinney, Michael D" , "devel@edk2.groups.io" , "lersek@redhat.com" Cc: "Feng, Bob C" , "Gao, Liming" Message-ID: <156520696314.20973.15022549262963633561@jljusten-skl> User-Agent: alot/0.8 Date: Wed, 07 Aug 2019 12:42:43 -0700 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2019-08-05 15:01:14, Kinney, Michael D wrote: > Laszlo, >=20 > The context of this change is only to the PatchCheck.py tool. > and how that tool uses git show. >=20 > I agree with the summary of very flexible capabilities in git > to help developers review different types of files. All of=20 > 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=20 > 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=20 > 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 > My expectation is that EDK II Maintainers need to review=20 > 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).