Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revoke the previous approved breaking changes if ARM request changes after the previous approval #7709

Open
qiaozha opened this issue Feb 20, 2024 · 3 comments
Assignees
Labels
Breaking Changes Improvements to Breaking Changes tooling Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.

Comments

@qiaozha
Copy link
Member

qiaozha commented Feb 20, 2024

As title, sometimes, service team get ARM signed off from a long time ago and SDK breaking changes review happens soon after that, but cases are common that service team could still need to change something after all the signed offs and ARM will revoke the ARM sign off labels, at that time, we should revoke all the approved SDK breaking changes as well.

See Azure/azure-rest-api-specs#24625 for an example. the previous invalid SDK breaking change approve labels cause we fail to run CI check properly, and the PR even get merged with all the SDK generation failed.

@qiaozha qiaozha changed the title Revoke the previous approved breaking changes if ARM request changes after the prevous approval Revoke the previous approved breaking changes if ARM request changes after the previous approval Feb 20, 2024
@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 20, 2024
@raych1
Copy link
Member

raych1 commented Feb 20, 2024

@konrad-jamrozik ,

I think the case is same as the API breaking change review and the ARM review. If there are new commits pushed to the PR which has ARM signed off and API/SDK breaking change approved, the corresponding approval labels should be revoked, and the reviewers need to review the new commits.

@konrad-jamrozik
Copy link
Contributor

@konrad-jamrozik konrad-jamrozik self-assigned this Feb 20, 2024
@konrad-jamrozik konrad-jamrozik added Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. labels Feb 20, 2024
@github-actions github-actions bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 20, 2024
@konrad-jamrozik konrad-jamrozik moved this from 🤔 Triage to 📋 Backlog in Azure SDK EngSys 🚢🎉 Feb 20, 2024
@konrad-jamrozik konrad-jamrozik moved this to 📋 Backlog in Spec PR Tools Feb 20, 2024
@konrad-jamrozik konrad-jamrozik added the Breaking Changes Improvements to Breaking Changes tooling label Jun 6, 2024
@qiaozha
Copy link
Member Author

qiaozha commented Jul 26, 2024

another example, service team make a lot of new changes after the initial approval, and ARM reviewer removed the SDK approved labels, however the bot added it back. Azure/azure-rest-api-specs#26059
image

which causes some of the SDK breaking changes has not really been reviewed properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Changes Improvements to Breaking Changes tooling Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo.
Projects
Status: 📋 Backlog
Status: 📋 Backlog
Development

No branches or pull requests

3 participants