From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.130753.1671100980723013380 for ; Thu, 15 Dec 2022 02:43:00 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=fWyFWo8p; spf=permerror, err=parse error for token &{10 18 %{ir}.%{v}.%{d}.spf.has.pphosted.com}: invalid domain name (domain: quicinc.com, ip: 205.220.180.131, mailfrom: quic_llindhol@quicinc.com) Received: from pps.filterd (m0279873.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2BFAeE8T013182; Thu, 15 Dec 2022 10:42:57 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=qcppdkim1; bh=aW5FlWi5lUV7ZYiFh8SNf4U0ODJcUydoYyWyh5JnFEI=; b=fWyFWo8p1WTms3YDRRebi3iT0/jOf1in7Q4DaJ55C+svIa6MNvtM1hx/MCpZbdFFEtS/ C0N23hr71Y6zWgKBAiD9tf2a018ySUWwX+EfCNRgTMBhcnFyPC1g3rT/MRokr8K/UgSY 0xZ96V7ZobDGic+dxfo8Ctcc8lsvLZtTPTN5D0HVqQclAKFal1Y4R004h3RGz5Xn23Ss 19xuwWAp9672ZLpPGVHtftrDfekHamLIc7zFM/M20YimvaNPSX+yhncHT+YbBB/WQfK8 OyAjH2m9IhpOyW0J9mqBdv9ZE6ceP8Iad+u1tPjPeGNfQYkyUxLYJH1LcxDvMv/xvvrJ CQ== Received: from nasanppmta05.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3mfxse8fx1-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 15 Dec 2022 10:42:57 +0000 Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 2BFAguFG003273 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 15 Dec 2022 10:42:56 GMT Received: from qc-i7.hemma.eciton.net (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.36; Thu, 15 Dec 2022 02:42:54 -0800 Date: Thu, 15 Dec 2022 10:42:51 +0000 From: "Leif Lindholm" To: Michael Kubacki CC: , , Ard Biesheuvel , Sami Mujawar , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH v3 13/14] ArmPkg: Turn off spellcheck audit mode Message-ID: References: <20221214225252.1660-1-mikuback@linux.microsoft.com> <20221214225252.1660-14-mikuback@linux.microsoft.com> <9c88fe54-5e83-fb43-b845-af4a79eaef79@linux.microsoft.com> <910eccca-b219-86ae-5550-339650b69461@linux.microsoft.com> MIME-Version: 1.0 In-Reply-To: <910eccca-b219-86ae-5550-339650b69461@linux.microsoft.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: MsG7gFSl1uJnA54TtzmtFwG0FLTKKTmo X-Proofpoint-GUID: MsG7gFSl1uJnA54TtzmtFwG0FLTKKTmo X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-12-15_05,2022-12-14_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 adultscore=0 clxscore=1015 spamscore=0 phishscore=0 priorityscore=1501 malwarescore=0 suspectscore=0 mlxscore=0 bulkscore=0 lowpriorityscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2212150083 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline 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. > Checking in typos creates more review work, makes the code history have typo > fix patches that could be avoided, and impacts accessibility. > > I spend far more time with edk2 overhead such as email formatting problems, > keeping track of maintainer email address changes when updating patches, > mapping email replies back to code, and so on that does not improve the > code. A spell checker only takes seconds and is built into edk2 CI. We didn't disable the spellchecker for the ARM* packages (only) because we hate correct spelling. We disabled it because it throws false positives left right and centre. So we end up needing to update the .ci.yaml every time we mention an architectural concept we haven't mentioned before. Or add a copyright line mentioning a new organisation. Or correctly use apostrophes in ways the spellchecker doesn't expect. Here is the current (and very incomplete, oh - and package specific) exception list for ArmPkg: https://github.com/tianocore/edk2/blob/master/ArmPkg/ArmPkg.ci.yaml#L95 If it had a aggressiveness knob and we could turn it down from 11 to 3 and work our way up from there, that would be another story. Failing that, a push-through-anyway-this-isn't-a-typo label would be a reasonable compromise. But for now, I agree with Ard. / Leif > On 12/14/2022 6:24 PM, Ard Biesheuvel wrote: > > On Thu, 15 Dec 2022 at 00:21, Michael Kubacki > > wrote: > > > > > > Yes. It will also reduce frequency of incoming patches that must be > > > reviewed and merged due to people continuously fixing trivial spelling > > > errors. > > > > > > > In that case, NAK to this patch (and the ArmVirtPkg one). Unless we > > add a button to the GitHub UI that permits me to override a negative > > CI result on a PR. > > > > > On 12/14/2022 6:07 PM, Ard Biesheuvel wrote: > > > > On Wed, 14 Dec 2022 at 23:53, wrote: > > > > > > > > > > From: Michael Kubacki > > > > > > > > > > Audit mode was enabled for the spellcheck CI plugin. It is no longer > > > > > needed with recent changes. Spelling errors can be checked in the > > > > > package moving forward. > > > > > > > > > > Cc: Leif Lindholm > > > > > Cc: Ard Biesheuvel > > > > > Cc: Sami Mujawar > > > > > Signed-off-by: Michael Kubacki > > > > > > > > Will this patch result in PRs potentially being rejected by pre-merge > > > > CI due to trivial spelling errors? > > > > > > > > > > > > > --- > > > > > ArmPkg/ArmPkg.ci.yaml | 3 ++- > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml > > > > > index c8dface6821a..a304c7966cf7 100644 > > > > > --- a/ArmPkg/ArmPkg.ci.yaml > > > > > +++ b/ArmPkg/ArmPkg.ci.yaml > > > > > @@ -87,7 +87,7 @@ > > > > > > > > > > ## options defined .pytool/Plugin/SpellCheck > > > > > "SpellCheck": { > > > > > - "AuditOnly": True, > > > > > + "AuditOnly": False, > > > > > "IgnoreFiles": [ > > > > > "Library/ArmSoftFloatLib/berkeley-softfloat-3/**" > > > > > ], # use gitignore syntax to ignore errors > > > > > @@ -148,6 +148,7 @@ > > > > > "fcmplt", > > > > > "ffreestanding", > > > > > "frsub", > > > > > + "hauser", > > > > > "hisilicon", > > > > > "iccabpr", > > > > > "iccbpr", > > > > > -- > > > > > 2.28.0.windows.1 > > > > > > > > > > > > >