From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 4C4CE941BA8 for ; Thu, 16 Nov 2023 17:36:58 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=/Lhaue9kOD2sy9tGbB4kdK6uhIBV5ZDPZdpxm74qEMY=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1700156217; v=1; b=wEHscDxZkVGlSbsFQIuMwGSPPzypsGfoXJ1U4M58COUCKkY8S9Kwc9nPPL91cIJAaucC5hlD XKVoVoGRUW8hBa38Cgs1WGzBZvxtFw90eZ0xRGla5rhxKEBUWN/mreqnJ3oZJd7jfS9t+H9riSS loDQWRquYU/Hk8JCJdwxgIw4= X-Received: by 127.0.0.2 with SMTP id OGtrYY7687511xgJlYpNP8UL; Thu, 16 Nov 2023 09:36:57 -0800 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.12984.1700156216345772517 for ; Thu, 16 Nov 2023 09:36:56 -0800 X-Received: from [192.168.4.22] (unknown [47.201.241.95]) by linux.microsoft.com (Postfix) with ESMTPSA id 18D0020B74C0; Thu, 16 Nov 2023 09:36:55 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 18D0020B74C0 Message-ID: <7cf7eacc-fc9e-4cad-9757-758e3bbf64dd@linux.microsoft.com> Date: Thu, 16 Nov 2023 12:36:53 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] edk2 uncrustify update (73.0.8)? To: devel@edk2.groups.io, pedro.falcato@gmail.com, Laszlo Ersek Cc: Rebecca Cran , Michael Kinney , Andrew Fish , Marcin Juszkiewicz , "Leif Lindholm (Quic)" References: <0107c96b-849a-db48-194b-1a4c1f3b0c78@redhat.com> <23dd696a-52a1-4c26-bfb6-5b5587325c42@linux.microsoft.com> <30e9b11c-ab39-4266-8981-5242542b625d@bsdio.com> From: "Michael Kubacki" In-Reply-To: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mikuback@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: exuGpna3f5M8c2vuiyOnYSeUx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=wEHscDxZ; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 11/16/2023 3:29 AM, Pedro Falcato wrote: > On Tue, Nov 14, 2023 at 3:01=E2=80=AFPM Laszlo Ersek = wrote: >> >> On 11/13/23 22:33, Pedro Falcato wrote: >>> On Mon, Nov 13, 2023 at 8:37=E2=80=AFPM Rebecca Cran wrote: >>>> >>>> On 11/13/2023 1:08 PM, Michael Kubacki wrote: >>>>> Yes. I just did it. It is relatively minor and impacts expected code >>>>> areas. >>>>> >>>>> https://github.com/tianocore/edk2/pull/5043/files >>>> >>>> >>>> Could you update .git-blame-ignore-revs please? >>> >>> You can't do that until the merge is done, since we use >>> rebase-and-merge for tianocore (and rebases do not keep stable commit >>> hashes). >>> But I would plead that this should not get merged in general :/ >> >=20 > Laszlo! >=20 > Sorry for the delay. >=20 >> Seeing the cumulative diff in that PR, do you have specific >> counter-arguments? >=20 > Well, my counter-argument is that formatting is becoming a topic of > its own. I used to be very pro-formatter but if this leads to either > 1) eyesore or 2) weird churn every now and then, > I feel like we should reconsider the current approach. I feel like all > formatting (manual or automated) is fine as long as it's: > 1) Visually consistent with the codebase's style > 2) Not horrendous to look at >=20 > and switching back and forth because 'magic indentation tool' says so > just seems silly to me. >=20 This is the first time Uncrustify has been updated in edk2 since Dec 7,=20 2021. https://github.com/tianocore/edk2/commits/master/.pytool/Plugin/UncrustifyC= heck/uncrustify_ext_dep.yaml Its configuration has also not changed during that time. For this=20 update, as Laszlo said, it is a trivial update to a few files. Can you elaborate on "switching back and forth"? >> >> The diff is trivial, IMO. You mentioned "error prone" and "much churn", >> which are very valid concerns, but they don't seem to apply here. We can >> review a diff of this size (especially if it's split up on Pkg >> boundaries), and the github view indicates the change is only in >> whitespace amount. >> >> The change in >> "OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c" >> is a net win; the current formatting is really distracting. >> >> Furthermore, this diff actually highlights some inexplicable syntax in >> "EmulatorPkg/FvbServicesRuntimeDxe/FvbInfo.c": those backslashes (not >> parts of any macro definition) are an eyesore. They should be fixed >> regardless of re-uncrustification. >> >> The version N vs. N+1 concern shouldn't be one; the authoritative >> version is what the YAML file in edk2 says. >=20 > Well, I fear it's not that simple. EDK2's uncrustify has historically > been a PITA, I've had to convince people to give it a try, I myself > don't even know how I installed it (IIRC, some weird random > combination of unzip + the NuGet...). > Getting everyone on the same version would already be hard even if the > software was easy to install. >=20 As I mentioned before, stuart makes installation simple. Simple meaning=20 it connects to the NuGet feed, pulls the NuGet package, and=20 automatically uses the appropriate build for your host OS when running=20 other commands. The installation instructions are here:=20 https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formattin= g#installing-uncrustify If you don't want to run stuart, you just need to click the link in=20 manual instructions and unzip to get the executable. This should only=20 take a couple minutes as shown here. https://gist.githubusercontent.com/makubacki/ed07d7fab53b1f68b28742126dd76b= 55/raw/11310b59f867e217f06fbc11339f4e18f2247fe5/HowToDownloadUncrustifyLinu= x.gif --- I'm not a full time tools developer or anything so I don't have a lot of=20 time to spend on this but I truly do want to reduce pain points if there=20 are small tweaks to the process that can be made. -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111322): https://edk2.groups.io/g/devel/message/111322 Mute This Topic: https://groups.io/mt/102559740/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-