[edk2-devel] github impact:breaking-change

[edk2-devel] github impact:breaking-change

[Leif Lindholm]

Hi,

I'm seeing something in several PRs in flight (and merged) that have 
"impact:breaking-change" set, where the purpose of the PR is to fix said 
breakage, not introduce API compatibilities.

Am I correct in my understanding that this is not the intended use, and 
if so how do we address the misconception?
Can we start by adding a description for the label? It currently has none.

Mu uses "Requires integration attention", which matches my 
understanding, but feels a bit abstract. How about
"This change breaks existing APIs"
or
"This change may require corresponding updates to code in other 
repositories"?

/
     Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119813): https://edk2.groups.io/g/devel/message/119813
Mute This Topic: https://groups.io/mt/107100295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] github impact:breaking-change

[Michael Kubacki]

Hi Leif,

Thanks for raising this. The label was intended to be set from the PR 
template which has an explanation:

https://github.com/tianocore/edk2/blob/master/.github/pull_request_template.md

If the breaking change box is checked, it will be added automatically.

I added a description in GitHub for the label as well:

https://github.com/tianocore/edk2/labels

Thanks,
Michael

On 7/8/2024 7:00 AM, Leif Lindholm wrote:
> Hi,
> 
> I'm seeing something in several PRs in flight (and merged) that have 
> "impact:breaking-change" set, where the purpose of the PR is to fix said 
> breakage, not introduce API compatibilities.
> 
> Am I correct in my understanding that this is not the intended use, and 
> if so how do we address the misconception?
> Can we start by adding a description for the label? It currently has none.
> 
> Mu uses "Requires integration attention", which matches my 
> understanding, but feels a bit abstract. How about
> "This change breaks existing APIs"
> or
> "This change may require corresponding updates to code in other 
> repositories"?
> 
> /
>      Leif


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119849): https://edk2.groups.io/g/devel/message/119849
Mute This Topic: https://groups.io/mt/107100295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] github impact:breaking-change

[Leif Lindholm]

Hi Michael,

On Tue, Jul 09, 2024 at 10:44:46 -0400, Michael Kubacki wrote:
> Hi Leif,
> 
> Thanks for raising this. The label was intended to be set from the PR
> template which has an explanation:
> 
> https://github.com/tianocore/edk2/blob/master/.github/pull_request_template.md
> 
> If the breaking change box is checked, it will be added automatically.

Right, so it seems some people have misunderstood the description in
the template.

The current text is
"Breaking change - Will this cause a break in build or boot behavior?"

I raised https://github.com/tianocore/edk2/pull/5895 for some minor
language tweaks that hopefully reduce the risk of misinterpretation.

> I added a description in GitHub for the label as well:
> 
> https://github.com/tianocore/edk2/labels

Thanks!

Regards,

Leif

> 
> Thanks,
> Michael
> 
> On 7/8/2024 7:00 AM, Leif Lindholm wrote:
> > Hi,
> > 
> > I'm seeing something in several PRs in flight (and merged) that have
> > "impact:breaking-change" set, where the purpose of the PR is to fix said
> > breakage, not introduce API compatibilities.
> > 
> > Am I correct in my understanding that this is not the intended use, and
> > if so how do we address the misconception?
> > Can we start by adding a description for the label? It currently has none.
> > 
> > Mu uses "Requires integration attention", which matches my
> > understanding, but feels a bit abstract. How about
> > "This change breaks existing APIs"
> > or
> > "This change may require corresponding updates to code in other
> > repositories"?
> > 
> > /
> >      Leif
> 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119851): https://edk2.groups.io/g/devel/message/119851
Mute This Topic: https://groups.io/mt/107100295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] github impact:breaking-change

[Michael Kubacki]

The tweaks look good to me.

Thanks,
Michael

On 7/9/2024 11:03 AM, Leif Lindholm wrote:
> Hi Michael,
> 
> On Tue, Jul 09, 2024 at 10:44:46 -0400, Michael Kubacki wrote:
>> Hi Leif,
>>
>> Thanks for raising this. The label was intended to be set from the PR
>> template which has an explanation:
>>
>> https://github.com/tianocore/edk2/blob/master/.github/pull_request_template.md
>>
>> If the breaking change box is checked, it will be added automatically.
> 
> Right, so it seems some people have misunderstood the description in
> the template.
> 
> The current text is
> "Breaking change - Will this cause a break in build or boot behavior?"
> 
> I raised https://github.com/tianocore/edk2/pull/5895 for some minor
> language tweaks that hopefully reduce the risk of misinterpretation.
> 
>> I added a description in GitHub for the label as well:
>>
>> https://github.com/tianocore/edk2/labels
> 
> Thanks!
> 
> Regards,
> 
> Leif
> 
>>
>> Thanks,
>> Michael
>>
>> On 7/8/2024 7:00 AM, Leif Lindholm wrote:
>>> Hi,
>>>
>>> I'm seeing something in several PRs in flight (and merged) that have
>>> "impact:breaking-change" set, where the purpose of the PR is to fix said
>>> breakage, not introduce API compatibilities.
>>>
>>> Am I correct in my understanding that this is not the intended use, and
>>> if so how do we address the misconception?
>>> Can we start by adding a description for the label? It currently has none.
>>>
>>> Mu uses "Requires integration attention", which matches my
>>> understanding, but feels a bit abstract. How about
>>> "This change breaks existing APIs"
>>> or
>>> "This change may require corresponding updates to code in other
>>> repositories"?
>>>
>>> /
>>>       Leif
>>
>>
>> 
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119852): https://edk2.groups.io/g/devel/message/119852
Mute This Topic: https://groups.io/mt/107100295/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-