[edk2-devel] [PATCH v3 13/14] ArmPkg: Turn off spellcheck audit mode

Fri Jan 6 02:46:48 UTC 2023

Hi Ard,

I believe you're referencing the points in this mail, right?
https://edk2.groups.io/g/devel/message/97433

To be clear about why that was considered dismissive and passive 
aggressive, these sentences:

"I would also like to point out that all this focus on code aesthetics
that do not contribute to the object code at all is distracting from
things that would actually improve the safety and robustness of the
code."

"So if you are interested, and want to improve EDK2 at a level that
actually makes the shipping code more robust, we could collaborate on
this (PE spec change, UEFI spec change, EDK2 and Linux changes both
for x86 and arm64[0]) so that firmware runs with indirect branch
protection enabled when the hardware supports it."

Read as "this is unimportant/distracting so do this more important stuff 
instead".

This may not been what you meant to convey, but it doesn't set the best 
foundation for collaboration on the topic.

I understand that there will always be functionally impacting work that 
has tangible value over supporting changes like correctly spelling 
words. Though, it's not purely aesthetic, language translators and 
screen readers function more reliably if words are spelled correctly. In 
any case, improving reading comprehension and functionality are not 
mutually exclusive.

I think we're meeting on the topic with two different sets of 
expectations that naturally conflict.

1. You're not happy with the current process used for spell checking in 
TianoCore
2. I would like to prevent and/or minimize future patches that only fix 
typos (check in CI)
3. We both believe fixing spelling errors should be simple

(1) is a valid opinion to have but because of (1), I cannot enable (2). 
Because of (1), (3) is not true as the infrastructure needs to be 
revamped, which I do not personally have bandwidth to do at the moment.

My initial understanding was that the TianoCore process had further 
discussion and agreement than it seems occurred so I thought we (as a 
community) were in position to simply follow it and reduce the typo 
count. Without (2), the typos fixed can reappear (the next commit could 
reintroduce the same typo without enforcement) so, in my opinion, it's 
not worth the time and effort to manage the patch series in that case. 
That's why I revoked the series.

Your feedback is still valuable to the wider TianoCore Tools & CI group 
that meets weekly. It may be able to be addressed there.

On the topic of IBT/BTI, I agree that's useful. We probably need to have 
a dedicated thread on that to get more background on where you're at and 
then we can try to help where possible.

Would you be open to a meeting to discuss that?

Thanks,
Michael

On 1/4/2023 5:37 AM, Ard Biesheuvel wrote:
> On Thu, 15 Dec 2022 at 17:38, Michael Kubacki
> <mikuback at linux.microsoft.com> wrote:
>>
>> On 12/15/2022 5:42 AM, Leif Lindholm wrote:
>>> On Wed, Dec 14, 2022 at 19:04:06 -0500, Michael Kubacki wrote:
>>>> I'm just trying to understand your position.
>>>>
>>>> Are you saying you would rather people check in typos and then later have
>>>> patches come into the package to fix them?
>>>>
>>>> For example, like these:
>>>>
>>>> - ArmVirtPkg: https://edk2.groups.io/g/devel/message/97021
>>>> - ArmPkg: https://edk2.groups.io/g/devel/message/97022
>>>>
>>>> Why not just have the code checked in without typos in the first place?
>>>
>>> A little fairy once whispered in my ear that if I stopped myself and
>>> tried to rephrase whenever I found myself using the work "just", I
>>> would meet less friction in context-stripping communication mediums
>>> such as email. They weren't wrong.
>>>
>>
>> This topic is met with friction regardless of how it is phrased.
>>
>> The friction exists because the community chose to enable spell checking
>> in CI and it is not wanted here.
>>
>> The mechanism chosen to ignore words was through YAML files rather than
>> a button. A common YAML file can store words for the whole project and
>> packages can add package-specific words. The community was expected to
>> make the contributions necessary in the common file to minimize impact
>> on package maintainers.
>>
>> The spellcheck log outputs the exact code that needs to be copied/pasted
>> to the exception list - whether the global list or package list. If you
>> run CI locally, you can copy/paste the exact lines needed.
>>
>> This is a patch series I work on in my spare time to try to improve the
>> project. I am tired of Ard's dismissive and passive aggressive responses
>> such as those in https://edk2.groups.io/g/devel/message/97433 so I
>> revoke the series and will let others decide what they want to do.
>>
> 
> I don't think this is a fair characterization of my response. I
> explained in detail why I think rigid spellcheck rules are
> counter-productive and do not contribute to the quality of what gets
> deployed to devices.
> 
> Nevertheless, I apologize if my frustration with the recent CI changes
> managed to seep through, although I should also mention that I am a
> big fan of the pre-merge build and boot tests, as they have been a
> huge help in my workflow. Only the rigid enforcement of standards that
> are purely cosmetic is what bothers me, especially because finding the
> error messages (and suggestions for improvement) in the complex UI is
> not as straight-forward as it is made out to be.
> 
> That said, it would be helpful if you could respond to the actual
> points I made, rather than dismissing them wholesale as a
> passive-aggressive and hostile response. You have presented your
> contribution as a take-it-or-leave-it style change, rather than
> engaging with Leif and me as the package maintainers to converge on
> something that we can all agree on.
> 
> You may have also missed my question/invitation regarding
> collaboration on IBT/BTI enablement in UEFI.
> 
> Kind regards,
> Ard.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#98049): https://edk2.groups.io/g/devel/message/98049
Mute This Topic: https://groups.io/mt/95678218/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-