ArmPkg/ArmPkg.ci.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Uncrustify checks are too rigid, making them counter-productive:
- it leads to code that is arguably harder to parse visually (e.g.,
the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
429309e0c6b74792)
- it forces indentation-only changes to code in the vicinity of actual
changes, making the code history more bloated than necessary (see
commit 7f198321eec0f520373 for an example)
- finding out from the web UI what exactly Uncrustify objected to is not
straight-forward.
So let's enable AuditMode for ArmPkg, so that interested parties can see
the uncrustify recommendations if desired, but without preventing the
changes from being merged. This leaves it at the discretion of the
ArmPkg maintainers to decide which level of conformance is required.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
ArmPkg/ArmPkg.ci.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
index 24db7425051388cf..d3124816118944cb 100644
--- a/ArmPkg/ArmPkg.ci.yaml
+++ b/ArmPkg/ArmPkg.ci.yaml
@@ -239,5 +239,10 @@
],
"AdditionalIncludePaths": [] # Additional paths to spell check
# (wildcards supported)
+ },
+
+ # options defined in .pytool/Plugin/UncrustifyCheck
+ "UncrustifyCheck": {
+ "AuditOnly": True
}
}
--
2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105640): https://edk2.groups.io/g/devel/message/105640
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Are there particular areas that could be improved to make it more usable
for you? I'm trying to find actionable improvements that can be made, if
any.
I know it's not perfect but developers can run it with a single keyboard
shortcut and it's been useful internally for eliminating style minutia
from distracting design and correctness conversation in code reviews.
On 6/2/2023 4:51 AM, Ard Biesheuvel wrote:
> Uncrustify checks are too rigid, making them counter-productive:
>
> - it leads to code that is arguably harder to parse visually (e.g.,
> the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
> 429309e0c6b74792)
Looking at commit 7f198321eec0f520373, I see positive changes like in
ArmCrashDumpDxe.c spacing in the ASSERT calls and treatment of
multi-line parameter formatting in the mCpu->RegisterInterruptHandler()
call are consistent.
That carries on for a number of C calls with inconsistent spacing before
opening parentheses and calls such as those to the DEBUG macro which I
find easier to read separating each parameter on a dedicated line.
In AArch64Mmu.h, I agree that preserving the (mostly) global column as
opposed to block-specific columns would be easier to vertically scan. Is
that the main issue in the file?
> - it forces indentation-only changes to code in the vicinity of actual
> changes, making the code history more bloated than necessary (see
> commit 7f198321eec0f520373 for an example)
That's true. People adjusted things like indentation before depending on
a given change, but it is predetermined now. Perhaps turning off column
alignment (in general or in a given package) could help reduce noise
from this.
I don't think alignment enforcement is necessary. That might also help
address some of thrash in AArch64Mmu.h.
> - finding out from the web UI what exactly Uncrustify objected to is not
> straight-forward.
>
This should not be necessary. It can be run locally to produce the same
result as in CI.
IDE-specific, but a lot of people use VS Code with the Uncrustify
extension
(https://marketplace.visualstudio.com/items?itemName=zachflower.uncrustify)
and that allows Uncrustify to be run against the open source file by
simply pressing the "code formatter" command.
I usually write the code, stage or commit it and then run this command
on the file. The code is formatted and it gives a normal local diff of
exactly what Uncrustify changed.
> So let's enable AuditMode for ArmPkg, so that interested parties can see
> the uncrustify recommendations if desired, but without preventing the
> changes from being merged. This leaves it at the discretion of the
> ArmPkg maintainers to decide which level of conformance is required.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> ArmPkg/ArmPkg.ci.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
> index 24db7425051388cf..d3124816118944cb 100644
> --- a/ArmPkg/ArmPkg.ci.yaml
> +++ b/ArmPkg/ArmPkg.ci.yaml
> @@ -239,5 +239,10 @@
> ],
>
> "AdditionalIncludePaths": [] # Additional paths to spell check
>
> # (wildcards supported)
>
> + },
>
> +
>
> + # options defined in .pytool/Plugin/UncrustifyCheck
>
> + "UncrustifyCheck": {
>
> + "AuditOnly": True
>
> }
>
> }
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105659): https://edk2.groups.io/g/devel/message/105659
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Fri, 2 Jun 2023 at 17:26, Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> Are there particular areas that could be improved to make it more usable
> for you? I'm trying to find actionable improvements that can be made, if
> any.
>
> I know it's not perfect but developers can run it with a single keyboard
> shortcut and it's been useful internally for eliminating style minutia
> from distracting design and correctness conversation in code reviews.
>
I don't disagree with that. But I just don't find it as important as
other people seem to feel it is, and combined with the flaky CI infra
(I just had to amend and push another PR [0] due to infra failure), I
am finding that just getting changes merged is taking a substantial
amount of time, which I'd prefer to spend on other things.
The coding style that uncrustify imposes is overly rigid, and leaves
no room at all for any discretion on the part of the maintainer.
[0] https://github.com/tianocore/edk2/pull/4470
>
> On 6/2/2023 4:51 AM, Ard Biesheuvel wrote:
> > Uncrustify checks are too rigid, making them counter-productive:
> >
> > - it leads to code that is arguably harder to parse visually (e.g.,
> > the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
> > 429309e0c6b74792)
>
> Looking at commit 7f198321eec0f520373, I see positive changes like in
> ArmCrashDumpDxe.c spacing in the ASSERT calls and treatment of
> multi-line parameter formatting in the mCpu->RegisterInterruptHandler()
> call are consistent.
>
> That carries on for a number of C calls with inconsistent spacing before
> opening parentheses and calls such as those to the DEBUG macro which I
> find easier to read separating each parameter on a dedicated line.
>
That may be true. But I personally don't think it is important enough
to a) rigidly enforce, and b) fix up for existing code (but that ship
sailed when we did the mass conversion)
> In AArch64Mmu.h, I agree that preserving the (mostly) global column as
> opposed to block-specific columns would be easier to vertically scan. Is
> that the main issue in the file?
>
Yes.
> > - it forces indentation-only changes to code in the vicinity of actual
> > changes, making the code history more bloated than necessary (see
> > commit 7f198321eec0f520373 for an example)
>
> That's true. People adjusted things like indentation before depending on
> a given change, but it is predetermined now. Perhaps turning off column
> alignment (in general or in a given package) could help reduce noise
> from this.
>
Note that I find this aspect quite important. In my opinion, the git
history *is* the code base. Everything we changed at any point in the
past, including the commit logs, is part of this, and mass whitespace
conversions, or spurious changes just to make the CI happy are
problematic to me. For this reason alone, I should be able to override
CI choices at my discretion as a maintainer.
> I don't think alignment enforcement is necessary. That might also help
> address some of thrash in AArch64Mmu.h.
>
Interesting. If there is room for configuration here, why does the
existing configuration deviate from the coding style guidelines we had
been using for years? AIUI, we use a private fork of uncrustify for
edk2, right? Are there any problematic aspects in the existing
guidelines that was difficult to automate?
> > - finding out from the web UI what exactly Uncrustify objected to is not
> > straight-forward.
> >
>
> This should not be necessary. It can be run locally to produce the same
> result as in CI.
>
> IDE-specific, but a lot of people use VS Code with the Uncrustify
> extension
> (https://marketplace.visualstudio.com/items?itemName=zachflower.uncrustify)
> and that allows Uncrustify to be run against the open source file by
> simply pressing the "code formatter" command.
>
> I usually write the code, stage or commit it and then run this command
> on the file. The code is formatted and it gives a normal local diff of
> exactly what Uncrustify changed.
>
I'm sure this all seems quite reasonable if you already bought into
using uncrustify. But for a drive-by contributor, or someone like me
who has been contributing code for many years based on the agreed
coding style guidelines, I struggle to understand why uncrustify is a
reasonable solution to the problem of inconsistent coding style.
I'm all for legible code, don't get me wrong. (And Leif is much more
pedantic in that sense than I am). But in my experience, the
uncrustify rules are too rigid, and arbitrary (two spaces indentation
here, four spaces there, etc etc)
I suppose having a button in the GitHub UI that simply lets us
exercise our discretion as maintainers to deviate from these rules
would go a long way in addressing these concerns. But as it stands
now, having my carefully crafted contributions rejected by an
automated system based on guidelines that deviate from the ones that
we agreed to, without any recourse, is simply not acceptable.
> > So let's enable AuditMode for ArmPkg, so that interested parties can see
> > the uncrustify recommendations if desired, but without preventing the
> > changes from being merged. This leaves it at the discretion of the
> > ArmPkg maintainers to decide which level of conformance is required.
> >
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> > Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > ArmPkg/ArmPkg.ci.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
> > index 24db7425051388cf..d3124816118944cb 100644
> > --- a/ArmPkg/ArmPkg.ci.yaml
> > +++ b/ArmPkg/ArmPkg.ci.yaml
> > @@ -239,5 +239,10 @@
> > ],
> >
> > "AdditionalIncludePaths": [] # Additional paths to spell check
> >
> > # (wildcards supported)
> >
> > + },
> >
> > +
> >
> > + # options defined in .pytool/Plugin/UncrustifyCheck
> >
> > + "UncrustifyCheck": {
> >
> > + "AuditOnly": True
> >
> > }
> >
> > }
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105660): https://edk2.groups.io/g/devel/message/105660
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi, > > In AArch64Mmu.h, I agree that preserving the (mostly) global column as > > opposed to block-specific columns would be easier to vertically scan. Is > > that the main issue in the file? > > Yes. It is possible to exclude specific files from checks, see CryptoPkg for example which does this for some files generated by openssl configure. > I'm sure this all seems quite reasonable if you already bought into > using uncrustify. But for a drive-by contributor, or someone like me > who has been contributing code for many years based on the agreed > coding style guidelines, I struggle to understand why uncrustify is a > reasonable solution to the problem of inconsistent coding style. I find it quite nice. Yes, you have to invest the time to set things up once. But then you can just delegate all code formating to uncrustify instead of doing it manually. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106254): https://edk2.groups.io/g/devel/message/106254 Mute This Topic: https://groups.io/mt/99282612/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 6/2/2023 11:50 AM, Ard Biesheuvel wrote:
> On Fri, 2 Jun 2023 at 17:26, Michael Kubacki
> <mikuback@linux.microsoft.com> wrote:
>>
>> Are there particular areas that could be improved to make it more usable
>> for you? I'm trying to find actionable improvements that can be made, if
>> any.
>>
>> I know it's not perfect but developers can run it with a single keyboard
>> shortcut and it's been useful internally for eliminating style minutia
>> from distracting design and correctness conversation in code reviews.
>>
>
> I don't disagree with that. But I just don't find it as important as
> other people seem to feel it is, and combined with the flaky CI infra
> (I just had to amend and push another PR [0] due to infra failure), I
> am finding that just getting changes merged is taking a substantial
> amount of time, which I'd prefer to spend on other things.
>
> The coding style that uncrustify imposes is overly rigid, and leaves
> no room at all for any discretion on the part of the maintainer.
>
> [0] https://github.com/tianocore/edk2/pull/4470
>
>>
>> On 6/2/2023 4:51 AM, Ard Biesheuvel wrote:
>>> Uncrustify checks are too rigid, making them counter-productive:
>>>
>>> - it leads to code that is arguably harder to parse visually (e.g.,
>>> the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
>>> 429309e0c6b74792)
>>
>> Looking at commit 7f198321eec0f520373, I see positive changes like in
>> ArmCrashDumpDxe.c spacing in the ASSERT calls and treatment of
>> multi-line parameter formatting in the mCpu->RegisterInterruptHandler()
>> call are consistent.
>>
>> That carries on for a number of C calls with inconsistent spacing before
>> opening parentheses and calls such as those to the DEBUG macro which I
>> find easier to read separating each parameter on a dedicated line.
>>
>
> That may be true. But I personally don't think it is important enough
> to a) rigidly enforce, and b) fix up for existing code (but that ship
> sailed when we did the mass conversion)
>
Yeah, (b) is done and enforcement can be opted by package for this reason.
>> In AArch64Mmu.h, I agree that preserving the (mostly) global column as
>> opposed to block-specific columns would be easier to vertically scan. Is
>> that the main issue in the file?
>>
>
> Yes.
>
Thanks for confirming.
>>> - it forces indentation-only changes to code in the vicinity of actual
>>> changes, making the code history more bloated than necessary (see
>>> commit 7f198321eec0f520373 for an example)
>>
>> That's true. People adjusted things like indentation before depending on
>> a given change, but it is predetermined now. Perhaps turning off column
>> alignment (in general or in a given package) could help reduce noise
>> from this.
>>
>
> Note that I find this aspect quite important. In my opinion, the git
> history *is* the code base. Everything we changed at any point in the
> past, including the commit logs, is part of this, and mass whitespace
> conversions, or spurious changes just to make the CI happy are
> problematic to me. For this reason alone, I should be able to override
> CI choices at my discretion as a maintainer.
>
I think there's a balance to not have wildly inconsistent style (reason
the guidelines were introduced in the first place), reduce maintainer
overhead for style only suggestions, and keep a clean history. But I
completely agree with the importance of git history.
>> I don't think alignment enforcement is necessary. That might also help
>> address some of thrash in AArch64Mmu.h.
>>
>
> Interesting. If there is room for configuration here, why does the
> existing configuration deviate from the coding style guidelines we had
> been using for years? AIUI, we use a private fork of uncrustify for
> edk2, right? Are there any problematic aspects in the existing
> guidelines that was difficult to automate?
>
Yes, there were several. Some not entirely related to the guidelines but
common implementation patterns. Multi-line parameter indentation in
function prototypes was not supported. Special handling of double
parentheses in DEBUG macro calls was formatted in very weird ways.
Inconsistent use of OPTIONAL before and after commas in parameter lists
led to varying outcomes. Inline assembly formatting just didn't work
well at all (compared to what was already in the code base) and was
disabled. The edk2 configuration is in [1].
[1] -
https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/UncrustifyCheck/uncrustify.cfg
>>> - finding out from the web UI what exactly Uncrustify objected to is not
>>> straight-forward.
>>>
>>
>> This should not be necessary. It can be run locally to produce the same
>> result as in CI.
>>
>> IDE-specific, but a lot of people use VS Code with the Uncrustify
>> extension
>> (https://marketplace.visualstudio.com/items?itemName=zachflower.uncrustify)
>> and that allows Uncrustify to be run against the open source file by
>> simply pressing the "code formatter" command.
>>
>> I usually write the code, stage or commit it and then run this command
>> on the file. The code is formatted and it gives a normal local diff of
>> exactly what Uncrustify changed.
>>
>
> I'm sure this all seems quite reasonable if you already bought into
> using uncrustify. But for a drive-by contributor, or someone like me
> who has been contributing code for many years based on the agreed
> coding style guidelines, I struggle to understand why uncrustify is a
> reasonable solution to the problem of inconsistent coding style.
>
> I'm all for legible code, don't get me wrong. (And Leif is much more
> pedantic in that sense than I am). But in my experience, the
> uncrustify rules are too rigid, and arbitrary (two spaces indentation
> here, four spaces there, etc etc)
>
> I suppose having a button in the GitHub UI that simply lets us
> exercise our discretion as maintainers to deviate from these rules
> would go a long way in addressing these concerns. But as it stands
> now, having my carefully crafted contributions rejected by an
> automated system based on guidelines that deviate from the ones that
> we agreed to, without any recourse, is simply not acceptable.
>
I understand and long time contributors generally done a good job
maintaining the style. It is difficult to suggest delta to contributor
only changes in future commits if specific code does not meet the rules
in prior commits though.
>
>
>
>>> So let's enable AuditMode for ArmPkg, so that interested parties can see
>>> the uncrustify recommendations if desired, but without preventing the
>>> changes from being merged. This leaves it at the discretion of the
>>> ArmPkg maintainers to decide which level of conformance is required.
>>>
>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>> ArmPkg/ArmPkg.ci.yaml | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
>>> index 24db7425051388cf..d3124816118944cb 100644
>>> --- a/ArmPkg/ArmPkg.ci.yaml
>>> +++ b/ArmPkg/ArmPkg.ci.yaml
>>> @@ -239,5 +239,10 @@
>>> ],
>>>
>>> "AdditionalIncludePaths": [] # Additional paths to spell check
>>>
>>> # (wildcards supported)
>>>
>>> + },
>>>
>>> +
>>>
>>> + # options defined in .pytool/Plugin/UncrustifyCheck
>>>
>>> + "UncrustifyCheck": {
>>>
>>> + "AuditOnly": True
>>>
>>> }
>>>
>>> }
>>>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105665): https://edk2.groups.io/g/devel/message/105665
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Ard,
Are there particular areas that could be improved to make it more usable
for you? I'm trying to find actionable improvements that can be made, if
any.
I know it's not perfect but developers can run it with a single keyboard
shortcut and it's been useful internally for eliminating style minutia
from distracting design and correctness conversation in code reviews.
> - finding out from the web UI what exactly Uncrustify objected to is not
> straight-forward.
>
This should not be necessary. It can be run locally to produce the same
result as in CI.
IDE-specific, but a lot of people use VS Code with the Uncrustify
extension
(https://marketplace.visualstudio.com/items?itemName=zachflower.uncrustify)
and that allows Uncrustify to be run against the open source file by
simply pressing the "code formatter" command.
I usually write the code stage or commit it and then run this command on
the file. The code is formatted and it gives a normal local diff of
exactly what Uncrustify changed.
On 6/2/2023 4:51 AM, Ard Biesheuvel wrote:
> Uncrustify checks are too rigid, making them counter-productive:
>
> - it leads to code that is arguably harder to parse visually (e.g.,
> the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
> 429309e0c6b74792)
Looking at commit 7f198321eec0f520373, I see positive changes like in
ArmCrashDumpDxe.c spacing in the ASSERT calls and treatment of
multi-line parameter formatting in the mCpu->RegisterInterruptHandler()
call are consistent.
That carries on for a number of C calls with inconsistent spacing before
opening parentheses and calls such as those to the DEBUG macro which I
find easier to read separating each parameter on a dedicated line.
In AArch64Mmu.h, I agree that preserving the (mostly) global column as
opposed to block-specific columns would be easier to vertically scan. Is
that the main issue in the file?
> - it forces indentation-only changes to code in the vicinity of actual
> changes, making the code history more bloated than necessary (see
> commit 7f198321eec0f520373 for an example)
> - finding out from the web UI what exactly Uncrustify objected to is not
> straight-forward.
>
> So let's enable AuditMode for ArmPkg, so that interested parties can see
> the uncrustify recommendations if desired, but without preventing the
> changes from being merged. This leaves it at the discretion of the
> ArmPkg maintainers to decide which level of conformance is required.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> ArmPkg/ArmPkg.ci.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
> index 24db7425051388cf..d3124816118944cb 100644
> --- a/ArmPkg/ArmPkg.ci.yaml
> +++ b/ArmPkg/ArmPkg.ci.yaml
> @@ -239,5 +239,10 @@
> ],
>
> "AdditionalIncludePaths": [] # Additional paths to spell check
>
> # (wildcards supported)
>
> + },
>
> +
>
> + # options defined in .pytool/Plugin/UncrustifyCheck
>
> + "UncrustifyCheck": {
>
> + "AuditOnly": True
>
> }
>
> }
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105657): https://edk2.groups.io/g/devel/message/105657
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Fri, Jun 02, 2023 at 10:51:36 +0200, Ard Biesheuvel wrote:
> Uncrustify checks are too rigid, making them counter-productive:
>
> - it leads to code that is arguably harder to parse visually (e.g.,
> the changes to ArmPkg/Include/Chipset/AArch64Mmu.h in commit
> 429309e0c6b74792)
> - it forces indentation-only changes to code in the vicinity of actual
> changes, making the code history more bloated than necessary (see
> commit 7f198321eec0f520373 for an example)
> - finding out from the web UI what exactly Uncrustify objected to is not
> straight-forward.
>
> So let's enable AuditMode for ArmPkg, so that interested parties can see
> the uncrustify recommendations if desired, but without preventing the
> changes from being merged. This leaves it at the discretion of the
> ArmPkg maintainers to decide which level of conformance is required.
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
I'm going to merge this - at least for now - since some of the quirks
are holding back merging code that is compliant because of existing
things it takes an issue with elsewhere in the file.
/
Leif
> ---
> ArmPkg/ArmPkg.ci.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
> index 24db7425051388cf..d3124816118944cb 100644
> --- a/ArmPkg/ArmPkg.ci.yaml
> +++ b/ArmPkg/ArmPkg.ci.yaml
> @@ -239,5 +239,10 @@
> ],
> "AdditionalIncludePaths": [] # Additional paths to spell check
> # (wildcards supported)
> + },
> +
> + # options defined in .pytool/Plugin/UncrustifyCheck
> + "UncrustifyCheck": {
> + "AuditOnly": True
> }
> }
> --
> 2.39.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105643): https://edk2.groups.io/g/devel/message/105643
Mute This Topic: https://groups.io/mt/99282612/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.