From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web11.2399.1652818031377040449 for ; Tue, 17 May 2022 13:07:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=VkMOVdZP; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 30981B81C1C for ; Tue, 17 May 2022 20:07:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E19E1C34100 for ; Tue, 17 May 2022 20:07:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1652818027; bh=OnZfZNcO0jYQk97+ANHn/bmxP7vtAM3oLb/wQcWYIPY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=VkMOVdZPLEd5LTiruWoRGjzF996W2IRo2tXtTxhjuc+Z+xcz7QZ2GmKhlYyCMneJa 4lbasYSK6Moz5h9otZvy9+N9AoS+xcLUnxEkv9a1U6hxm8XsiaGj1tQXmebZqZ0q9F n/JBFx6KJn7MnoPXI8mMoN+fZDDP/RVw77J3TfFEKoib1jvu6i1gA1uaGj/OQix4Kv pD10O4jS7LY/W/mzmHAYayLmDODwO1s/iJ9/80OSNrxNfuMAOiegsBGuDha8nRIbEF lXttcOSuBsra/UNVmQkrQZTczh0EOFVOIwuFrlMat74O6bYR21CnVt5nw6FD7UAyWp 4HaV2iwOpc2ug== Received: by mail-oi1-f172.google.com with SMTP id w130so219852oig.0 for ; Tue, 17 May 2022 13:07:07 -0700 (PDT) X-Gm-Message-State: AOAM533yZZFVE4mrXVOOeEKjj6saZcM74DIOqueYZvfLFYmDavBlVBkM TBOCDZO2TwPvFTLVh9CyLemQpv81rH6MR6Y50fg= X-Google-Smtp-Source: ABdhPJy/LI5ZgDeSOYgWJQpfMs6VrmlPb7/Sdtyd310AXY41X+nCiWcc1v++Z857JkjeUh4voGN61Hzk8FyF5QfNczQ= X-Received: by 2002:a05:6808:1314:b0:326:e438:d8cd with SMTP id y20-20020a056808131400b00326e438d8cdmr17217067oiv.228.1652818027042; Tue, 17 May 2022 13:07:07 -0700 (PDT) MIME-Version: 1.0 References: <20220517160043.1210-1-mikuback@linux.microsoft.com> <52bf72a9-604e-a1ca-8846-34a7801ef427@linux.microsoft.com> <8d950da7-9e1a-d5ff-6baf-6aeeca36736a@linux.microsoft.com> In-Reply-To: <8d950da7-9e1a-d5ff-6baf-6aeeca36736a@linux.microsoft.com> From: "Ard Biesheuvel" Date: Tue, 17 May 2022 22:06:55 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v1 0/8] Fix new typos reported To: Michael Kubacki Cc: edk2-devel-groups-io , Alexei Fedorov , Ankit Sinha , Ard Biesheuvel , Bret Barkelew , Gerd Hoffmann , Guomin Jiang , Jiewen Yao , Leif Lindholm , Liming Gao , Michael D Kinney , Nate DeSimone , Ray Ni , Sami Mujawar , Sean Brogan , Wei6 Xu Content-Type: text/plain; charset="UTF-8" On Tue, 17 May 2022 at 21:32, Michael Kubacki wrote: > > 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. > If you feel it is worth your time to fix typos in existing comments, I won't stand in your way. But I don't feel it is worth my time, given that it doesn't actually improve the code, except for by some artifical measure of spelling-correctness, which has no bearing at all on what runs on people's machines, and as far as I can tell, these typoes do not create any confusion regarding what the comments intend to convey. Adding typoed words to the ignorelist is the worst possible solution, because you will be wasting your time only to placate the machine, accumulating technical debt in the code base without actually fixing the problems. So that is out of the question for me. If you want to fix these issues, that is also fine. I will review/ack with priority provided that I actually have any bandwidth available. But if we are working for the CI instead of the other way around, something is seriously wrong. If we can't roll a stable tag because the CI wants us to fix our typoes first, we have to be able to override it. And corrupting the codebase by adding typoes to the ignorelist just to placate the CI is preposterous..