Improve ARM PR template and guidance around breaking changes (#25086)

Azure · Aug 2, 2023 · 2eb9a4d · 2eb9a4d
1 parent cc92777
commit 2eb9a4d
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 32 deletions.
diff --git a/.github/PULL_REQUEST_TEMPLATE/control_plane_template.md b/.github/PULL_REQUEST_TEMPLATE/control_plane_template.md
@@ -31,25 +31,27 @@ What's the purpose of this PR? Check all that apply. This is **mandatory**!
 To merge this PR, you **must** go through the following checklist and confirm you understood 
 and followed the instructions by checking all the boxes:
 
-- [ ] I have reviewed the general guidance on the spec PR review process: https://aka.ms/specprreview.
-- [ ] I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data-plane related specifications.
-- [ ] I commit to follow the [Breaking Change Policy](https://aka.ms/AzBreakingChangesPolicy).
+- [ ] I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
 - [ ] I have reviewed following [Resource Provider guidelines](https://aka.ms/rpguidelines), including
   [ARM resource provider contract](https://github.com/Azure/azure-resource-manager-rpc) and
   [REST guidelines](https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md) (estimated time: 4 hours).  
-  I understand this is required before I can request review from an ARM API Review board.
+  I understand this is required before I can proceed to Step 2, "ARM Review", for this PR.
 
-### ARM API changes review
+### Breaking changes review (Step 1)
 
-- If you want for the ARM team to review this PR, you must add the `ARMReview` label. 
-- The automation may automatically add the `ARMReview` label, if appropriate.  
-  If this happens, proceed according to guidance given in GitHub comments also added by the automation.
+- If the automation determines you have breaking changes, i.e. Step 1 from the diagram applies to you,
+  you must follow the [breaking changes process].  
+  **IMPORTANT** This applies even if:
+  - The tool fails while it shouldn't, e.g. due to runtime exception, or incorrect detection of breaking changes.
+  - You believe there is no need for you to request breaking change approval, for any reason. 
+    Such claims must be reviewed, and the process is the same.
 
-### Breaking change review
+### ARM API changes review (Step 2)
+
+- If this PR is in purview of ARM review then automation will add the `ARMReview` label.
+- If you want to force ARM review, add the label yourself.
+- Proceed according to the diagram at the top of this comment.
 
-If you have any breaking changes as defined in the [Breaking Change Policy](https://aka.ms/AzBreakingChangesPolicy/), 
-follow the process outlined in the [High-level Breaking Change Process doc](https://aka.ms/breakingchangesprocess#high-level-breaking-change-process).
-
 ## Getting help
 
 - For general PR approval workflow, see the diagram at the top of this comment.
@@ -62,3 +64,4 @@ follow the process outlined in the [High-level Breaking Change Process doc](http
 [List of SDK breaking changes approvers]: https://teams.microsoft.com/l/message/19:0351f5f9404446e4b4fd4eaf2c27448d@thread.skype/1689115217750?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=3e17dcb0-4257-4a30-b843-77f47f1d4121&parentMessageId=1689115217750&teamName=Azure%20SDK&channelName=API%20Spec%20Review&createdTime=1689115217750
 [public repo merge queue]: https://github.com/Azure/azure-rest-api-specs/pulls?q=is%3Aopen+is%3Apr+label%3AMergeRequested+-label%3AIDCDevDiv+draft%3Afalse+sort%3Acreated-asc
 [private repo merge queue]: https://github.com/Azure/azure-rest-api-specs-pr/pulls?q=is%3Aopen+is%3Apr+label%3AMergeRequested+-label%3AIDCDevDiv+draft%3Afalse+sort%3Acreated-asc
+[breaking changes process]: https://eng.ms/docs/cloud-ai-platform/azure-core/azure-core-pm-and-design/trusted-platform-pm-karimb/service-lifecycle-and-actions-team/service-lifecycle-actions-team/apex/media/launchingproductbreakingchanges#breaking-change-process-1
diff --git a/.github/comment.yml b/.github/comment.yml
@@ -14,15 +14,16 @@
       <br/><br/>
       **ACTION ITEM ALTERNATIVE B**: Request approval. <br/>
       Alternatively, if you cannot fix the breaking changes, then you can request an approval for them.
-      Please follow the process described in the
-      [High-level Breaking Change Process doc](https://aka.ms/breakingchangesprocess#high-level-breaking-change-process).
-      <br/><br/>
-      **ACTION ITEM ALTERNATIVE C**: Report false positive. <br/>
-      If you think there are no breaking changes, 
-      i.e. the validation should pass yet it fails, 
-      then proceed as explained in **ACTION ITEM ALTERNATIVE B**.<br/>
-      This applies even if the breaking change tool fails with internal runtime error. 
-      In such case a manual breaking change review is necessary.
+      Please follow the [breaking changes process](https://eng.ms/docs/cloud-ai-platform/azure-core/azure-core-pm-and-design/trusted-platform-pm-karimb/service-lifecycle-and-actions-team/service-lifecycle-actions-team/apex/media/launchingproductbreakingchanges#breaking-change-process-1).
+      <br/>
+      This case applies even if:
+      <ul>
+      <li>The tool fails while it shouldn't, e.g. due to runtime exception, or incorrect detection of breaking changes.
+      </li>
+      <li>You believe there is no need for you to request breaking change approval, for any reason. 
+      Such claims must be reviewed, and the process is the same.
+      </li>
+      </ul>
 
 - rule:
     type: label
@@ -43,17 +44,16 @@
       **ACTION ITEM ALTERNATIVE B**: Request approval. <br/>
       Alternatively, if you cannot avoid modifying existing API versions, 
       then you can request an approval for them.
-      Please follow the process described in the
-      [High-level Breaking Change Process doc](https://aka.ms/breakingchangesprocess#high-level-breaking-change-process).
-      <br/><br/>
-      **ACTION ITEM ALTERNATIVE C**: Report false positive. <br/>
-      If you think there are no changes in existing API version, 
-      i.e. there should be no `NewApiVersionRequired` label,
-      then proceed as explained in **ACTION ITEM ALTERNATIVE B**.<br/>
-      This applies even if the breaking change tool fails with internal runtime error. 
-      In such case a manual breaking change review is necessary.
-      <br/><br/>   
-      For additional guidance, please see https://aka.ms/NewApiVersionRequired
+      Please follow the [breaking changes process](https://eng.ms/docs/cloud-ai-platform/azure-core/azure-core-pm-and-design/trusted-platform-pm-karimb/service-lifecycle-and-actions-team/service-lifecycle-actions-team/apex/media/launchingproductbreakingchanges#breaking-change-process-1).
+      <br/>
+      This case applies even if:
+      <ul>
+      <li>The tool fails while it shouldn't, e.g. due to runtime exception, or incorrect detection of breaking changes.
+      </li>
+      <li>You believe there is no need for you to request breaking change approval, for any reason. 
+      Such claims must be reviewed, and the process is the same.
+      </li>
+      </ul>
 
 - rule:
     type: label