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

["Laszlo Ersek" <lersek@redhat.com>]

On 11/13/23 20:07, Pedro Falcato wrote:
> On Mon, Nov 13, 2023 at 11:58 AM Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> Hi Michael,
>>
>> recently I encountered an uncrustify failure on github.
>>
>> The reason was that my local uncrustify was *more recent* (73.0.8) than
>> the one we use in edk2 CI (which is 73.0.3, per the edk2 file
>> ".pytool/Plugin/UncrustifyCheck/uncrustify_ext_dep.yaml").
> 
> Wait, you can use upstream uncrustify? I'm just using whatever
> uncrustify version I took from the project-mu fork...

Apologies -- for edk2 purposes (and I don't use uncrustify for anything
else), I consider
<https://projectmu@dev.azure.com/projectmu/Uncrustify/_git/Uncrustify>
"upstream".

> 
>>
>> Updating the version number in the YAML file (i.e., advancing edk2 to
>> version 73.0.8) seems easy enough, but:
>>
>> - Do you think 73.0.8 is mature enough for adoption in edk2?
>>
>>   This upstream uncrustify release was tagged in April (and I can't see
>>   any more recent commits), so I assume it should be stable.
>>
>> - Would the version update require a whole-tree re-uncrustification?
> 
> Please, no. I didn't mind doing an initial reformatting at first, but
> doing this continuously is both 1) problem-prone 2) just amazing
> amounts of churn.
> Let's say I have version N, you have version N+1 - we may never get
> any final, formatted output as your version formats it differently
> from mine.

Your argument against a continuously reformatted code base is well
received; just a small correction: we should never have an N vs. N+1
dilemma. What CI uses is the authoritative version.

> 
> I don't know how the CI is doing its thing atm (I haven't merged
> anything myself to edk2), but the uncrustify check should be relaxed
> to just a warning.

I don't think that's going to happen. Everybody ignores warnings when in
a rush, or when the same warnings pop up for the 10th time.

> There's nothing wrong with what my uncrustify
> version is formatting to, there's nothing wrong with yours either, and
> CI isn't necessarily wrong either.
> 
> And, to be fair, I already find uncrustify a large pain in the butt to
> use (requiring a custom fork really does not help), but I find the
> benefits worth it *locally*, as my coding style is also quite
> different from the NT-esque style.

Funnily enough, my stance is quite the opposite. I happen to disagree
with some patterns that uncrustify enforces, but I'm thankful that at
any given state of CI (= using any given version of uncrustify), we
can't have any more debates about patch formatting (that is, it's
especially its central nature that I like). I've found uncrustify
relatively easy to use locally, too.

All in all I'm not trying to upset the status quo, it's just a question
about a version bump, and how we'd deal with any fallout from that.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111200): https://edk2.groups.io/g/devel/message/111200
Mute This Topic: https://groups.io/mt/102559740/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-