Re: [edk2-devel] [PATCH v2 4/7] ArmVirtPkg.ci.yaml: Add debug macro exception

["Michael Kubacki" <mikuback@linux.microsoft.com>]

On 9/19/2023 10:52 AM, Leif Lindholm wrote:
> On 2023-09-19 15:46, Ard Biesheuvel wrote:
>> On Mon, 18 Sept 2023 at 22:35, Michael Kubacki
>> <mikuback@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?
> 

I looked here:
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

It would be great if we could explicitly state the process around each 
tag either there or linked from there.

Thanks,
Michael

> /
>      Leif
> 
>>> On 9/13/2023 1:10 PM, Ard Biesheuvel wrote:
>>>> On Wed, 13 Sept 2023 at 19:07, <mikuback@linux.microsoft.com> wrote:
>>>>>
>>>>> From: Michael Kubacki <michael.kubacki@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+tianocore@kernel.org>
>>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> Acked-by: Ard Biesheuvel <ardb@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 (#108848): https://edk2.groups.io/g/devel/message/108848
Mute This Topic: https://groups.io/mt/101341655/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-