On 2023-09-19 15:46, Ard Biesheuvel wrote:
On Mon, 18 Sept 2023 at 22:35, Michael Kubacki
<mikub...@linux.microsoft.com> wrote:
Thanks Ard. I still need a Reviewed-by on this patch to complete the series.
Actually, you don't but I suppose there is some difference of
interpretation here.
In the Linux community (where these tags originated), an ack from a
maintainer means 'no objections, feel free to apply this to code that
I maintain'. In this case, I am not 100% sure what the change means,
so I am not comfortable claiming that I reviewed it, but given that
this is your area of expertise rather than mine, I trust that you know
what you are doing. Hence the acked-by.
In other cases, I may rely on another person's expertise and their
reviewed-by to decide whether or not to merge something, rather than
review it myself. In this case, the acked-by is implied by the fact
that I decided to merge the PR (and therefore somewhat redundant). But
it comes down to the same thing.
If there is a Tianocore rule that says every patch needs a reviewed-by
from a maintainer, we should probably fix that. It is *not* the job of
a maintainer to review everything (and this wouldn't scale to begin
with) - rather, it is their job to ensure that everything that gets
merged has been reviewed sufficiently, either by themselves or by
other people they have confidence in.
I was under the impression we had this conversation a few years ago and
agreed a maintainer Ack was sufficient. Have we failed to document that
fact?
/
Leif
On 9/13/2023 1:10 PM, Ard Biesheuvel wrote:
On Wed, 13 Sept 2023 at 19:07, <mikub...@linux.microsoft.com> wrote:
From: Michael Kubacki <michael.kuba...@microsoft.com>
Adds a CI YAML entry to acknowledge a case where a macro is expanded
that contains a print specifier.
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
Cc: Leif Lindholm <quic_llind...@quicinc.com>
Cc: Sami Mujawar <sami.muja...@arm.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com>
Acked-by: Ard Biesheuvel <a...@kernel.org>
---
ArmVirtPkg/ArmVirtPkg.ci.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtPkg.ci.yaml b/ArmVirtPkg/ArmVirtPkg.ci.yaml
index 1e799dc4e194..506b0e72f0bb 100644
--- a/ArmVirtPkg/ArmVirtPkg.ci.yaml
+++ b/ArmVirtPkg/ArmVirtPkg.ci.yaml
@@ -125,5 +125,13 @@
], # words to extend to the dictionary for this package
"IgnoreStandardPaths": [], # Standard Plugin defined paths that
should be ignore
"AdditionalIncludePaths": [] # Additional paths to spell check
(wildcards supported)
+ },
+
+ "DebugMacroCheck": {
+ "StringSubstitutions": {
+ # DynamicTablesPkg/Include/ConfigurationManagerObject.h
+ # Reason: Expansion of macro that contains a print specifier.
+ "FMT_CM_OBJECT_ID": "0x%lx"
+ }
}
}
--
2.42.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108846): https://edk2.groups.io/g/devel/message/108846
Mute This Topic: https://groups.io/mt/101341655/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
