On 11/16/2023 3:29 AM, Pedro Falcato wrote:
On Tue, Nov 14, 2023 at 3:01 PM Laszlo Ersek <ler...@redhat.com> wrote:
On 11/13/23 22:33, Pedro Falcato wrote:
On Mon, Nov 13, 2023 at 8:37 PM Rebecca Cran <rebe...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
