From: "Pedro Falcato" <pedro.falcato@gmail.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: Rebecca Cran <rebecca@bsdio.com>,
devel@edk2.groups.io, mikuback@linux.microsoft.com,
Michael Kinney <michael.d.kinney@intel.com>,
Andrew Fish <afish@apple.com>,
Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>,
"Leif Lindholm (Quic)" <quic_llindhol@quicinc.com>
Subject: Re: [edk2-devel] edk2 uncrustify update (73.0.8)?
Date: Thu, 16 Nov 2023 08:29:28 +0000 [thread overview]
Message-ID: <CAKbZUD3r1wS+C7tjih4qymJUZrRVgq2qRuPu1xo5CY4WefF=Tw@mail.gmail.com> (raw)
In-Reply-To: <d3c1ad1b-411f-5a1a-fe1e-ede645e2ac45@redhat.com>
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.
>
> 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.
--
Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111304): https://edk2.groups.io/g/devel/message/111304
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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-11-16 8:29 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-13 11:58 [edk2-devel] edk2 uncrustify update (73.0.8)? Laszlo Ersek
2023-11-13 12:29 ` Marcin Juszkiewicz
2023-11-13 19:14 ` Rebecca Cran via groups.io
2023-11-13 20:37 ` Michael Kubacki
2023-11-13 19:07 ` Pedro Falcato
2023-11-13 20:21 ` Michael Kubacki
2023-11-13 21:05 ` Michael D Kinney
2023-11-14 14:51 ` Laszlo Ersek
2023-11-14 15:12 ` Rebecca Cran via groups.io
2023-11-15 8:52 ` Laszlo Ersek
[not found] ` <17974449E158DE38.1153@groups.io>
2023-11-13 19:10 ` Pedro Falcato
2023-11-13 20:08 ` Michael Kubacki
2023-11-13 20:37 ` Rebecca Cran
2023-11-13 21:33 ` Pedro Falcato
2023-11-14 15:01 ` Laszlo Ersek
2023-11-16 8:29 ` Pedro Falcato [this message]
2023-11-16 17:36 ` Michael Kubacki
2023-11-23 2:07 ` Pedro Falcato
2023-11-17 9:08 ` Laszlo Ersek
2023-11-23 1:44 ` Pedro Falcato
2023-11-14 1:46 ` Michael Kubacki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKbZUD3r1wS+C7tjih4qymJUZrRVgq2qRuPu1xo5CY4WefF=Tw@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox