From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web10.2330.1607392011883656194 for ; Mon, 07 Dec 2020 17:46:52 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Evft1BZ2; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607392011; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Gj1f7t/3g7mP6wHCSkBstSqoF85c0luE6+NzScWBkrw=; b=Evft1BZ2Ew5r5PObrI0WzEZydghAkmNUwcQkNJl4onmJUMox291yZf2mu3PZLFME6WMjG4 pNjM3P4He+K1IijSfHGBWTt5UZiX1wtEy47DKW1mAczdl+Zkze8LfyCIfgmWWr1pwOCyPP HHFSXTG19/h1Bwfk3cCBvDqiQYHaA1I= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-216-tCJ31LxuOvuYlU3NCQGCbw-1; Mon, 07 Dec 2020 20:46:45 -0500 X-MC-Unique: tCJ31LxuOvuYlU3NCQGCbw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 485E0804004; Tue, 8 Dec 2020 01:46:44 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-30.ams2.redhat.com [10.36.112.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6C94A5D9DE; Tue, 8 Dec 2020 01:46:42 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list To: Sean Brogan , devel@edk2.groups.io, Ard Biesheuvel Cc: James Bottomley , Jordan Justen , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , Tom Lendacky , "Leif Lindholm (Nuvia address)" References: <20201204032116.31321-1-lersek@redhat.com> <20201204032116.31321-2-lersek@redhat.com> <7c57b6dc-de1a-8f2b-9b40-c1faf9b46bfe@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 8 Dec 2020 02:46:41 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 12/04/20 19:28, Sean Brogan wrote: > I would also agree with Ard about shortening and simplifying the commit > message if this commit is to go forward. > > As a FYI the pytools issue you link to for ci comment is closed as "wont > fix".  That doesn't change the fact that Edk2 CI runs an edk2 plugin > that does potentially bad things to your local workspace and if your > environment is configured in unexpected ways the plugin causes even more > damage. > > More importantly instead of this commit i ask if the community should > have a quick value prop discussion of EccCheck and if in its current > form needs changes or to be disabled...then that would be the change > rather than this commit. - Disabling ECC globally: yes please. Much better idea than this patch. - "quick" value prop discussion: my fear was (and is) that it would not be "quick"; I foresee either a heated debate or a drawn-out indecisive process. Meanwhile good patches would still be held back in either case, which is the only thing I really care about here. So if "quick" is indeed quick, then I'm OK; I just figured the really quick way would be this patch. > I am generally a fan of automation and tools > based validation for code formatting but there has been a lot of noise > with this one so it might not yet be ready to be a PR blocker. Agreed. > > Personally, related to code formatting/conventions i would much much > rather see the community agree to a profile in clang-format or something > similar and then just run the tool on all files in the tree and commit > the changes.  This might mean we have to change a few things as i > haven't been able to get clang-format to match exactly...but in the end > auto formatting is in my opinion a better path forward than home grown > tools to "enforce" formatting.  Auto formatting could be easily enforced > in CI and is easy/nearly free for a contributor to resolve and help the > community create consistent code.  I know its not perfect but it gets > you 95% of the way without huge investment. I have no experience with auto-formatting. I'm not a fan of sweeping changes (huge conversion commits). It's unclear how the above would differ from ECC "policy wise" -- it would be a different set of formatting opinions, yes, but it would still be a tool to enforce them, with unclear options for exceptions. I'd suggest splitting the two: first let's disable ECC (or make it opt-in -- and not by metafile commits, but by github pull request labels!), and second investigate auto-formatting (also as an opt-in, so we could get a taste). Thanks! Laszlo