* Re: [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
2023-04-27 0:34 ` [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov Guo, Gua
@ 2023-04-27 0:52 ` Michael D Kinney
2023-04-27 1:23 ` Guo, Gua
2023-04-27 1:28 ` Michael Kubacki
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Michael D Kinney @ 2023-04-27 0:52 UTC (permalink / raw)
To: Guo, Gua, devel@edk2.groups.io
Cc: Sean Brogan, Michael Kubacki, Oliver Steffen, fernald, chris,
Kinney, Michael D
Hi Gua,
It is hard to review this type of patch without more context.
Can you provide a link to the PR or branch where the docker image was updated? That information should be part of the commit message.
Thanks,
Mike
> -----Original Message-----
> From: Guo, Gua <gua.guo@intel.com>
> Sent: Wednesday, April 26, 2023 5:34 PM
> To: devel@edk2.groups.io
> Cc: Guo, Gua <gua.guo@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Kubacki <mikuback@linux.microsoft.com>; Oliver Steffen
> <osteffen@redhat.com>; fernald, chris <chfernal@microsoft.com>
> Subject: [PATCH v2 2/2] .azurepipelines: Choose container that have installed
> lcov
>
> From: Gua Guo <gua.guo@intel.com>
>
> Azure should install code coverage tool (lcov), it didn't
> exist on Fedora and Ubuntu by default.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Oliver Steffen <osteffen@redhat.com>
> Cc: Chris Fernald <chfernal@microsoft.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
> .azurepipelines/templates/defaults.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.azurepipelines/templates/defaults.yml
> b/.azurepipelines/templates/defaults.yml
> index 74d6b41783..8412a43cad 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -9,4 +9,4 @@
>
>
> variables:
>
> default_python_version: ">=3.10.6"
>
> - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9"
>
> + default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:3b3eb8f"
>
> --
> 2.39.2.windows.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
2023-04-27 0:52 ` Michael D Kinney
@ 2023-04-27 1:23 ` Guo, Gua
0 siblings, 0 replies; 9+ messages in thread
From: Guo, Gua @ 2023-04-27 1:23 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Cc: Sean Brogan, Michael Kubacki, Oliver Steffen, fernald, chris,
Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]
Sure, I will update the commit message.
________________________________
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Thursday, April 27, 2023 8:52:40 AM
To: Guo, Gua <gua.guo@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki <mikuback@linux.microsoft.com>; Oliver Steffen <osteffen@redhat.com>; fernald, chris <chfernal@microsoft.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
Hi Gua,
It is hard to review this type of patch without more context.
Can you provide a link to the PR or branch where the docker image was updated? That information should be part of the commit message.
Thanks,
Mike
> -----Original Message-----
> From: Guo, Gua <gua.guo@intel.com>
> Sent: Wednesday, April 26, 2023 5:34 PM
> To: devel@edk2.groups.io
> Cc: Guo, Gua <gua.guo@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Kubacki <mikuback@linux.microsoft.com>; Oliver Steffen
> <osteffen@redhat.com>; fernald, chris <chfernal@microsoft.com>
> Subject: [PATCH v2 2/2] .azurepipelines: Choose container that have installed
> lcov
>
> From: Gua Guo <gua.guo@intel.com>
>
> Azure should install code coverage tool (lcov), it didn't
> exist on Fedora and Ubuntu by default.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Oliver Steffen <osteffen@redhat.com>
> Cc: Chris Fernald <chfernal@microsoft.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
> .azurepipelines/templates/defaults.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.azurepipelines/templates/defaults.yml
> b/.azurepipelines/templates/defaults.yml
> index 74d6b41783..8412a43cad 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -9,4 +9,4 @@
>
>
> variables:
>
> default_python_version: ">=3.10.6"
>
> - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9"
>
> + default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:3b3eb8f"
>
> --
> 2.39.2.windows.1
[-- Attachment #2: Type: text/html, Size: 3384 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
2023-04-27 0:34 ` [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov Guo, Gua
2023-04-27 0:52 ` Michael D Kinney
@ 2023-04-27 1:28 ` Michael Kubacki
2023-04-27 1:38 ` [edk2-devel] " Chris Fernald
2023-05-03 11:54 ` Oliver Steffen
3 siblings, 0 replies; 9+ messages in thread
From: Michael Kubacki @ 2023-04-27 1:28 UTC (permalink / raw)
To: gua.guo, devel
Cc: Michael D Kinney, Sean Brogan, Oliver Steffen, Chris Fernald
I agree with Mike that there should be more context in the commit
message about the change that added lcov installation to the container
(https://github.com/tianocore/containers/commit/f1c7a20f641852034b0d7e8c1b209407c92560f2).
Ideally, a summary of changes from the previous container version to
this one would be provided.
As for the container change itself, it looks fine and I'll add an R-b
now. As discussed, I think it would be nice to move to the Fedora 37
image when possible.
Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
On 4/26/2023 8:34 PM, gua.guo@intel.com wrote:
> From: Gua Guo <gua.guo@intel.com>
>
> Azure should install code coverage tool (lcov), it didn't
> exist on Fedora and Ubuntu by default.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Oliver Steffen <osteffen@redhat.com>
> Cc: Chris Fernald <chfernal@microsoft.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
> .azurepipelines/templates/defaults.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml
> index 74d6b41783..8412a43cad 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -9,4 +9,4 @@
>
>
> variables:
>
> default_python_version: ">=3.10.6"
>
> - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9"
>
> + default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:3b3eb8f"
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
2023-04-27 0:34 ` [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov Guo, Gua
2023-04-27 0:52 ` Michael D Kinney
2023-04-27 1:28 ` Michael Kubacki
@ 2023-04-27 1:38 ` Chris Fernald
2023-05-03 11:54 ` Oliver Steffen
3 siblings, 0 replies; 9+ messages in thread
From: Chris Fernald @ 2023-04-27 1:38 UTC (permalink / raw)
To: devel, gua.guo
Cc: Michael D Kinney, Sean Brogan, Michael Kubacki, Oliver Steffen,
Chris Fernald
Except the context previously mentioned by Mike and Michael, looks good
to me.
Reviewed-by: Chris Fernald <chfernal@microsoft.com>
On 4/26/2023 5:34 PM, Guo, Gua wrote:
> From: Gua Guo <gua.guo@intel.com>
>
> Azure should install code coverage tool (lcov), it didn't
> exist on Fedora and Ubuntu by default.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Oliver Steffen <osteffen@redhat.com>
> Cc: Chris Fernald <chfernal@microsoft.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
> .azurepipelines/templates/defaults.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml
> index 74d6b41783..8412a43cad 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -9,4 +9,4 @@
>
>
> variables:
>
> default_python_version: ">=3.10.6"
>
> - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9"
>
> + default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:3b3eb8f"
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov
2023-04-27 0:34 ` [PATCH v2 2/2] .azurepipelines: Choose container that have installed lcov Guo, Gua
` (2 preceding siblings ...)
2023-04-27 1:38 ` [edk2-devel] " Chris Fernald
@ 2023-05-03 11:54 ` Oliver Steffen
3 siblings, 0 replies; 9+ messages in thread
From: Oliver Steffen @ 2023-05-03 11:54 UTC (permalink / raw)
To: devel, gua.guo
Cc: Michael D Kinney, Sean Brogan, Michael Kubacki, Chris Fernald
We could also switch to Fedora 37, while we are already at it.
See: https://edk2.groups.io/g/devel/message/102947
But this patch looks fine to me.
Reviewed-by: Oliver Steffen <osteffen@redhat.com>
Quoting gua.guo@intel.com (2023-04-27 02:34:16)
> From: Gua Guo <gua.guo@intel.com>
>
> Azure should install code coverage tool (lcov), it didn't
> exist on Fedora and Ubuntu by default.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Oliver Steffen <osteffen@redhat.com>
> Cc: Chris Fernald <chfernal@microsoft.com>
> Signed-off-by: Gua Guo <gua.guo@intel.com>
> ---
> .azurepipelines/templates/defaults.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml
> index 74d6b41783..8412a43cad 100644
> --- a/.azurepipelines/templates/defaults.yml
> +++ b/.azurepipelines/templates/defaults.yml
> @@ -9,4 +9,4 @@
>
> variables:
> default_python_version: ">=3.10.6"
> - default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:47addc9"
> + default_linux_image: "ghcr.io/tianocore/containers/fedora-35-test:3b3eb8f"
> --
> 2.39.2.windows.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread