On Wed, 7 Sept 2022 at 04:37, <mikuback@...> wrote:

From: Michael Kubacki <michael.kubacki@...>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4046

Adds EmbeddedPkg to edk2 CI.

Due to a number of build errors (some intentional) such as
'BUILD_EPOCH' only being defined for GCC in
VirtualRealTimeClockLib.inf, the package is only run on GCC
build agents.

This still allows build to be verified and non-build CI checks
to be performed.

In the edk2 PR for this change, you can see that the package only
runs on GCC and CI passes with this configuration.

https://github.com/tianocore/edk2/pull/3299

Cc: Leif Lindholm <quic_llindhol@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Abner Chang <abner.chang@...>
Cc: Daniel Schaefer <git@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (3):
EmbeddedPkg/AcpiLib: Fix code formatting errors
EmbeddedPkg: Add CI YAML file
EmbeddedPkg: Only run in CI for GCC5

NAK until we have a discussion about strictness of CI and ways to
permit manual override of merge time CI restrictions.

EmbeddedPkg/Library/AcpiLib/AcpiLib.c | 88 +++++++++----------
.azurepipelines/templates/pr-gate-build-job.yml | 4 +
.pytool/CISettings.py | 1 +
EmbeddedPkg/EmbeddedPkg.ci.yaml | 89 ++++++++++++++++++++
EmbeddedPkg/EmbeddedPkg.dec | 8 ++
EmbeddedPkg/Include/Library/AcpiLib.h | 22 ++---
6 files changed, 158 insertions(+), 54 deletions(-)
create mode 100644 EmbeddedPkg/EmbeddedPkg.ci.yaml

--
2.28.0.windows.1

On Wed, 7 Sept 2022 at 17:00, Michael Kubacki
<mikuback@...> wrote:

When would you like to have that discussion?

The Tianocore Tool, CI, Codebase meeting is every week. In that meeting
we've discussed getting all edk2 packages to at least run CI.

https://github.com/tianocore/edk2/discussions/2614

If you prefer to have it here, that's fine as well.

In a nutshell, I am fine with enabling this, as long as I can override
the CI and merge PRs that the CI thinks have issues.

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Thursday, September 15, 2022 1:47 PM
To: Michael Kubacki <mikuback@...>
Cc: devel@edk2.groups.io; Leif Lindholm <quic_llindhol@...>; Ard Biesheuvel <ardb+tianocore@...>; Abner Chang
<abner.chang@...>; Daniel Schaefer <git@...>; Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [edk2-devel] [PATCH v1 0/3] EmbeddedPkg: Enable CI

On Thu, 15 Sept 2022 at 21:46, Michael Kubacki
<mikuback@...> wrote:

Hi Ard,

I haven't seen any action items for the v1 series.

Can you please check the series again and let me know if you have any
further concerns?

The only thing I'd like to know is how I can override the CI and merge
a PR that was rejected by the checks you are enabling here.

On 9/7/2022 11:16 AM, Ard Biesheuvel wrote:

On Wed, 7 Sept 2022 at 17:00, Michael Kubacki
<mikuback@...> wrote:

When would you like to have that discussion?

The Tianocore Tool, CI, Codebase meeting is every week. In that meeting
we've discussed getting all edk2 packages to at least run CI.

https://github.com/tianocore/edk2/discussions/2614

If you prefer to have it here, that's fine as well.

In a nutshell, I am fine with enabling this, as long as I can override
the CI and merge PRs that the CI thinks have issues.

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Thursday, September 15, 2022 2:03 PM
To: Kinney, Michael D <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>; devel@edk2.groups.io; Leif Lindholm <quic_llindhol@...>; Ard
Biesheuvel <ardb+tianocore@...>; Abner Chang <abner.chang@...>; Daniel Schaefer <git@...>
Subject: Re: [edk2-devel] [PATCH v1 0/3] EmbeddedPkg: Enable CI

On Thu, 15 Sept 2022 at 22:52, Kinney, Michael D
<michael.d.kinney@...> wrote:

Ard,

Why would you want to do that? The whole point of CI is to establish a minimum quality level for all code in the project.

They can be disabled with updates to the YAML file. Checks can be disabled completely and may of the checks support

exception lists.

If the only way to prevent this from happening is to turn it off again
in the YAML file, I'd prefer not to turn it on to begin with.

I agree that code quality is important, but IMO the checks we have at
the moment are way too strict, and 90% of the time I spend on
reviewing and merging patches is on crustify and patchcheck errors.
This is simply not worth my time.

Hi Ard,
If there is content you do not think needs to follow the min quality criteria, perhaps it can be moved out of edk2 repo? Maybe to edk2-staging or edk2-archive?
Thanks,
Mike

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Thursday, September 15, 2022 2:03 PM
To: Kinney, Michael D <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>; devel@edk2.groups.io; Leif Lindholm <quic_llindhol@...>; Ard
Biesheuvel <ardb+tianocore@...>; Abner Chang <abner.chang@...>; Daniel Schaefer <git@...>
Subject: Re: [edk2-devel] [PATCH v1 0/3] EmbeddedPkg: Enable CI

On Thu, 15 Sept 2022 at 22:52, Kinney, Michael D
<michael.d.kinney@...> wrote:

Ard,

Why would you want to do that? The whole point of CI is to establish a minimum quality level for all code in the project.

They can be disabled with updates to the YAML file. Checks can be disabled completely and may of the checks support

exception lists.

If the only way to prevent this from happening is to turn it off again
in the YAML file, I'd prefer not to turn it on to begin with.

I agree that code quality is important, but IMO the checks we have at
the moment are way too strict, and 90% of the time I spend on
reviewing and merging patches is on crustify and patchcheck errors.
This is simply not worth my time.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
Sent: Friday, September 23, 2022 4:47 AM
To: Michael Kubacki <mikuback@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; Leif Lindholm <quic_llindhol@...>; Ard Biesheuvel
<ardb+tianocore@...>; Abner Chang <abner.chang@...>; Daniel Schaefer <git@...>
Subject: Re: [edk2-devel] [PATCH v1 0/3] EmbeddedPkg: Enable CI

On Fri, 23 Sept 2022 at 03:09, Michael Kubacki
<mikuback@...> wrote:

What are the next steps?

As long as we are using the most lenient setting, I'm ok with this

Acked-by: Ard Biesheuvel <ardb@...>

On 9/15/2022 5:54 PM, Kinney, Michael D wrote:

Hi Ard,

If there is content you do not think needs to follow the min quality criteria, perhaps it can be moved out of edk2 repo? Maybe

to edk2-staging or edk2-archive?

Thanks,

Mike

-----Original Message-----
From: Ard Biesheuvel <ardb@...>
Sent: Thursday, September 15, 2022 2:03 PM
To: Kinney, Michael D <michael.d.kinney@...>
Cc: Michael Kubacki <mikuback@...>; devel@edk2.groups.io; Leif Lindholm <quic_llindhol@...>; Ard
Biesheuvel <ardb+tianocore@...>; Abner Chang <abner.chang@...>; Daniel Schaefer <git@...>
Subject: Re: [edk2-devel] [PATCH v1 0/3] EmbeddedPkg: Enable CI

On Thu, 15 Sept 2022 at 22:52, Kinney, Michael D
<michael.d.kinney@...> wrote:

Ard,

Why would you want to do that? The whole point of CI is to establish a minimum quality level for all code in the project.

They can be disabled with updates to the YAML file. Checks can be disabled completely and may of the checks support

exception lists.

If the only way to prevent this from happening is to turn it off again
in the YAML file, I'd prefer not to turn it on to begin with.

I agree that code quality is important, but IMO the checks we have at
the moment are way too strict, and 90% of the time I spend on
reviewing and merging patches is on crustify and patchcheck errors.
This is simply not worth my time.