Re: [edk2-devel] [PATCH v1 0/8] Fix new typos reported

["Michael Kubacki" <mikuback@linux.microsoft.com>]

As noted in the patch, this BZ was filed to follow up and review those:
https://bugzilla.tianocore.org/show_bug.cgi?id=3929

I don't like doing this either but the spelling errors do exist. I am 
trying not to make CI policy changes as those can be controversial even 
among maintainers in the same package and is an orthogonal conversation 
to addressing pre-existing issues within the presently defined CI policy.

In this specific case, the ignore list in the package CI YAML file can 
be used to explicitly identify known typos and the BZ explicitly tracks 
reviewing those so there's a well defined path to resolve and fix the issue.

I personally feel that's better than ignoring the problem entirely but 
it also depends on where your package code ends up getting consumed and 
the requirements and burden it might place on those consumers. For 
example, if it ends up in auto generated documentation and that 
documentation has spell check enabled, it becomes a downstream override.

There's currently several PRs active that fix typos so others see some 
value in this work (as opposed to disabling spell checking):
   - https://github.com/tianocore/edk2/pull/2900
   - https://github.com/tianocore/edk2/pull/2789
   - https://github.com/tianocore/edk2/pull/1955

For future changes, I suggested lock the cspell version and I think 
that's an option to prevent these from appearing at unknown points in 
time. I'm not appointed to make authoritative decisions about that (to 
my understanding) so I am making that suggestion for the community to 
consider.

Again, I don't have a strong opinion on this topic, I've been waiting 4 
weeks to get the v5 patch series merged (other busy work in between), 
and you're the maintainer. It sounds like if I take ownership of BZ 
3929, you might be okay with leaving it enabled? I can do that but 
there's so many words in this instance, I wanted someone closer to the 
package contents to look at it.

If you still strongly feel you would prefer to have it disabled, I will 
pull that change in and see if any opposing opinions surface. However, I 
wanted to double check this is what you want to do right now.

Thanks,
Michael

On 5/17/2022 1:31 PM, Ard Biesheuvel wrote:
> On Tue, 17 May 2022 at 18:25, Michael Kubacki
> <mikuback@linux.microsoft.com> wrote:
>>
>> Hi Ard,
>>
>> I think that's a reasonable approach.
>>
>> We could also consider locking onto a specific cspell version to
>> decrease the likelihood of this sporadically appearing in the future.
>>
>> In this case, I would prefer not to make the decision to disable spell
>> check entirely on behalf of various package maintainers though. I'm just
>> trying to keep the status quo from unblocking other changes.
>>
>> Do you think that's something you or others could add as a change on top
>> of this series?
>>
> 
> I guess that could wait until after the stable tag. However, your
> current series adds words such as "addressig" and "characteritics" to
> the ignorelist of ArmPkg, and that is something I do have a problem
> with: this is just busywork, as there is no confusion whatsoever about
> the meaning of the texts in question, nobody is proposing changes to
> the code that contains these typos, and adding typos to these
> ignorelists is clearly not the correct solution.
> 
> CI and automation are fine, but here, they are just creating problems,
> resulting in pointless churn, and impeding the stable tag process.
> Please, just turn it off.