From: "Sean" <spbrogan@outlook.com>
To: Laszlo Ersek <lersek@redhat.com>,
devel@edk2.groups.io, Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: "James Bottomley" <jejb@linux.ibm.com>,
"Jordan Justen" <jordan.l.justen@intel.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Tom Lendacky" <thomas.lendacky@amd.com>,
"Leif Lindholm (Nuvia address)" <leif@nuviainc.com>
Subject: Re: [edk2-devel] [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list
Date: Fri, 4 Dec 2020 10:28:13 -0800 [thread overview]
Message-ID: <DM6PR07MB7180771B7368CB34D16810D8C8F10@DM6PR07MB7180.namprd07.prod.outlook.com> (raw)
In-Reply-To: <7c57b6dc-de1a-8f2b-9b40-c1faf9b46bfe@redhat.com>
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. 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.
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.
Thanks
Sean
On 12/4/2020 7:36 AM, Laszlo Ersek wrote:
> Hi Sean,
>
> On 12/04/20 16:22, Laszlo Ersek wrote:
>> On 12/04/20 05:05, Sean Brogan wrote:
>
>>> 3. Running CI locally should not be "somewhat risky". More work needs
>>> to be done to identify the root cause of the above behavior but my guess
>>> is that it has to do with EccCheck and nothing to do with
>>> pytool-extensions.
>>
>> Sorry, I guess I mixed up my references a little bit. I consider running
>> binaries downloaded from the internet risky (except from the official
>> repos of my Linux distro(s)). But that's indeed a different topic and I
>> shouldn't have generalized. Sorry about that.
>
> If you have a suggestion to improve the wording here, I'd like to hear
> that. I'd really like to go ahead with this patch set in one way or
> another, as it's blocking James's work from being merged. I don't want
> to merge a commit message here that you find offensive or just plain
> wrong though, so please suggest an improvement.
>
> Ard, do you have any comments please?
>
> Thanks
> Laszlo
>
next prev parent reply other threads:[~2020-12-04 18:28 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-04 3:21 [PATCH 0/2] OvmfPkg: CI tweaks Laszlo Ersek
2020-12-04 3:21 ` [PATCH 1/2] OvmfPkg: start using the ECC plugin exception list Laszlo Ersek
2020-12-04 4:05 ` [edk2-devel] " Sean
2020-12-04 15:22 ` Laszlo Ersek
2020-12-04 15:36 ` Laszlo Ersek
2020-12-04 16:05 ` Ard Biesheuvel
2020-12-08 1:56 ` Laszlo Ersek
2020-12-08 2:10 ` James Bottomley
2020-12-08 7:05 ` Ard Biesheuvel
2020-12-08 18:45 ` Sean
2020-12-10 8:23 ` Laszlo Ersek
2020-12-04 18:28 ` Sean [this message]
2020-12-08 1:46 ` Laszlo Ersek
2020-12-04 3:21 ` [PATCH 2/2] OvmfPkg: add "gGrubFileGuid=Grub" to GuidCheck.IgnoreDuplicates Laszlo Ersek
2020-12-04 12:42 ` Philippe Mathieu-Daudé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR07MB7180771B7368CB34D16810D8C8F10@DM6PR07MB7180.namprd07.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox