From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lucky1.263xmail.com (lucky1.263xmail.com [211.157.147.135]) by mx.groups.io with SMTP id smtpd.web11.41.1652836747590896618 for ; Tue, 17 May 2022 18:19:08 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: byosoft.net, ip: 211.157.147.135, mailfrom: byomail@byosoft.net) Received: from localhost (unknown [192.168.167.69]) by lucky1.263xmail.com (Postfix) with ESMTP id DB252BF522 for ; Wed, 18 May 2022 09:19:03 +0800 (CST) X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-ADDR-CHECKED4: 1 X-SKE-CHECKED: 1 X-ABS-CHECKED: 1 X-ANTISPAM-LEVEL: 2 Received: from mail.byosoft.com.cn (unknown [58.240.74.242]) by smtp.263.net (postfix) whith ESMTP id P23899T140227399935744S1652836743202827_; Wed, 18 May 2022 09:19:03 +0800 (CST) X-IP-DOMAINF: 1 X-RL-SENDER: byomail@byosoft.net X-SENDER: byomail@byosoft.net X-LOGIN-NAME: byomail@byosoft.net X-FST-TO: devel@edk2.groups.io X-RCPT-COUNT: 1 X-LOCAL-RCPT-COUNT: 0 X-MUTI-DOMAIN-COUNT: 0 X-SENDER-IP: 58.240.74.242 X-ATTACHMENT-NUM: 0 X-UNIQUE-TAG: X-System-Flag: 0 Received: from DESKTOPS6D0PVI ([101.224.116.119]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Wed, 18 May 2022 09:18:06 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 101.224.116.119 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Michael Kubacki'" , , Cc: "'Alexei Fedorov'" , "'Ankit Sinha'" , "'Ard Biesheuvel'" , "'Bret Barkelew'" , "'Gerd Hoffmann'" , "'Guomin Jiang'" , "'Jiewen Yao'" , "'Leif Lindholm'" , "'Michael D Kinney'" , "'Nate DeSimone'" , "'Ray Ni'" , "'Sami Mujawar'" , "'Sean Brogan'" , "'Wei6 Xu'" References: <20220517160043.1210-1-mikuback@linux.microsoft.com> <52bf72a9-604e-a1ca-8846-34a7801ef427@linux.microsoft.com> <8d950da7-9e1a-d5ff-6baf-6aeeca36736a@linux.microsoft.com> <8acb2244-9262-7b27-a6c5-ff65dcdeda32@linux.microsoft.com> In-Reply-To: <8acb2244-9262-7b27-a6c5-ff65dcdeda32@linux.microsoft.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHYxIDAvOF0gRml4IG5ldyB0eXBvcyByZXBvcnRlZA==?= Date: Wed, 18 May 2022 09:18:08 +0800 Message-ID: <025101d86a55$22303a20$6690ae60$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHcLZTW2SO+N522U0ndsnapDIZJBgGCzCIMATngQ8oCIYs3VgIUJOhcAwYlZ4kCkTem2qy4AE0w Sender: "gaoliming" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Michael: Thanks for your quick work to resolve the CI issue. For this issue, if we= use the old stable version cspell version, those new issues will not be re= ported, right? If yes, can we update CI only to unblock PR first for this s= table tag? The change in Packages can be made in future.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Michael Kubacki > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B45=E6=9C=8818=E6=97=A5 = 7:51 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; ardb@kernel.org > =E6=8A=84=E9=80=81: 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 > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v1 0/8] Fix new typos reporte= d >=20 > Hi Ard, >=20 > I understand it is frustrating for things that were working to suddenly > stop and errors to have been missed by the plugin in the past. I'm also > surprised that some of these issues were previously not caught. >=20 > To clarify, adding the words to the ignore list was not really that much > time. The plugin output gives the words to add to the list (in JSON) so > that's a copy/paste operation and an IDE can remove duplicate lines > instantly so that was about a 10-30 second or so solution. Submitting > the BZ was another 1-2 minutes >=20 > Following the the edk2 contribution process to manually add maintainers > per package, rebase and manually add review tags, parse feedback inline > to unified diffs over email, generate patch files, and update the cover > letter was a relatively larger consumer of time. For v2, I took > ownership of the BZ and spent more time to try to reduce the likelihood > of unexpected issues appearing in the future. >=20 > V2 will do the following: > 1. Complete BZ 3929. > 2. Lock the cspell version to v5.20.0 to prevent latest from > unexpectedly causing issues in the future. > 3. Update the common word list in cspell.base.yaml to prevent package > level duplication in the future. > 4. Include Sami's code review tags. >=20 > I'm checking the CI results in the PR now and once it passes, I'll send > it on the list. >=20 > https://github.com/tianocore/edk2/pull/2903 >=20 > Thanks, > Michael >=20 > On 5/17/2022 4:06 PM, Ard Biesheuvel wrote: > > 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=3D3929 > >> > >> 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 eve= n > >> among maintainers in the same package and is an orthogonal conversatio= n > >> to addressing pre-existing issues within the presently defined CI poli= cy. > >> > >> 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 track= s > >> 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 wil= l > >> 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.. > > > > > >=20 > >