[Breaking Change] [ShiftLeft] ShiftLeft breaking change process on azure-rest-api-specs-pr is confusing and noisy
- Dominant language
- C#
- Stars
- 135
- Forks
- 260
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 143
Description
Per the email thread with subject `RE: AKS Breaking change review: Azure/azure-rest-api-specs-pr/pull/11872`
Relevant snippet from my email:
> Here is my current best guess how things work with my questions to you:
>
> (1) When a new PR is made against the private spec repo, [azure-rest-api-specs-pr](https://github.com/Azure/azure-rest-api-specs-pr/pull/9349), it is assumed that all the previous PRs have been published/released to the public repo, [azure-rest-api-specs](https://github.com/Azure/azure-rest-api-specs-pr/pull/9349). Hence if PR 1 added a new API to the private repo and PR 2 made breaking changes to that API in the private repo, it is assumed these are potentially actual breaking changes. [@Mike](mailto:mikekistler@microsoft.com), [@Jeffrey](mailto:jeffreyr@microsoft.com) is my understanding correct? Is this a good link to read more about this? https://aka.ms/azapi/breakingchanges (I got it from [@Jeffrey](mailto:jeffreyr@microsoft.com)’s signature [guidelines](https://github.com/microsoft/api-guidelines/tree/vNext/azure)).
>
> (2) However, ShiftLeft has a flow that violates rule (1). Instead, it treats the private repo as “work in progress”, where one can rapidly iterate on pending changes by pushing multiple PRs to the repo, under the assumption the previous PRs have not yet been published/released. The way it was handled by ShiftLeft is that its PRs were labelled “ShiftLeftApiSpecReview” and “BreakingChange” but not “BreakingChangeReviewRequired”. The idea here is that the PR reviewer would know that if the label “BreakingChangeReviewRequired” is missing and label “ShiftLeftApiSpecReview” is present, it means the “BreakingChange” label is bogus and needs to be ignored. [@Matthew Christopher](mailto:matthchr@microsoft.com) [@Ray Chen](mailto:raychen@microsoft.com) does this seem correct?
>
> The problem here is that the rule (2) requires memorizing this special case by PR reviewer and may lead to errors where we inadvertently approve breaking change because we though there wasn’t one, but there was. Ideally we want to get rid of this problem, either by moving the ShiftLeft iteration process off the default branches as [@Jeffrey](mailto:jeffreyr@microsoft.com) suggested, or perhaps have better labelling, like, if the automation detects it is just ShiftLeft changes being iterated on unreleased API, then no breaking changes should be reported by the tooling, at all. No warnings, no labels.
Contributor guide
Assessment
This issue has not been assessed yet.