Re: [edk2-devel] edk2 uncrustify update (73.0.8)?

["Michael Kubacki" <mikuback@linux.microsoft.com>]

On 11/16/2023 3:29 AM, Pedro Falcato wrote:
> On Tue, Nov 14, 2023 at 3:01 PM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 11/13/23 22:33, Pedro Falcato wrote:
>>> On Mon, Nov 13, 2023 at 8:37 PM Rebecca Cran <rebecca@bsdio.com> 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 :/
>>
> 
> Laszlo!
> 
> Sorry for the delay.
> 
>> Seeing the cumulative diff in that PR, do you have specific
>> counter-arguments?
> 
> 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
> 
> and switching back and forth because 'magic indentation tool' says so
> just seems silly to me.
> 
This is the first time Uncrustify has been updated in edk2 since Dec 7, 
2021.

https://github.com/tianocore/edk2/commits/master/.pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml

Its configuration has also not changed during that time. For this 
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.
> 
> 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.
> 
As I mentioned before, stuart makes installation simple. Simple meaning 
it connects to the NuGet feed, pulls the NuGet package, and 
automatically uses the appropriate build for your host OS when running 
other commands.

The installation instructions are here: 
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Code-Formatting#installing-uncrustify

If you don't want to run stuart, you just need to click the link in 
manual instructions and unzip to get the executable. This should only 
take a couple minutes as shown here.

https://gist.githubusercontent.com/makubacki/ed07d7fab53b1f68b28742126dd76b55/raw/11310b59f867e217f06fbc11339f4e18f2247fe5/HowToDownloadUncrustifyLinux.gif

---

I'm not a full time tools developer or anything so I don't have a lot of 
time to spend on this but I truly do want to reduce pain points if there 
are small tweaks to the process that can be made.


-=-=-=-=-=-=-=-=-=-=-=-
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]
-=-=-=-=-=-=-=-=-=-=-=-