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

APIView should somehow block PRs from being merged in GitHub #4375

Open
Tracked by #5760 ...
heaths opened this issue Oct 12, 2022 · 3 comments
Open
Tracked by #5760 ...

APIView should somehow block PRs from being merged in GitHub #4375

heaths opened this issue Oct 12, 2022 · 3 comments
Labels
APIView Central-EngSys This issue is owned by the Engineering System team.

Comments

@heaths
Copy link
Member

heaths commented Oct 12, 2022

With swagger/Cadl support coming online for APIView, I feel one important feature that's needed is to somehow block PRs on GitHub from being merged while there are unresolved comments. Ideally, showing those comments on appropriate lines in the PR would be nice so we're not forced to use separate tools- GitHub and APIView - but perhaps consider that a stretch or I can open another issue.

At the very least, perhaps some sort of hook to prevent merging while there are unresolved comments. I assume links will still be added to PRs so that service teams know to look at APIView, but randomized assignees for merging need to know why a PR is blocked and not be able to override e.g., a required check.

@heaths heaths added the APIView label Oct 12, 2022
@azure-sdk azure-sdk moved this to 🆕 New in ApiView Oct 12, 2022
@mikekistler mikekistler moved this from 🆕 New to 📋 Backlog in ApiView Nov 7, 2022
@mikekistler
Copy link
Member

Maybe APIView should add an "approved" label on the PR when approved in APIView.

@heaths
Copy link
Member Author

heaths commented Nov 7, 2022

That could work. We already have a convention of using green "Approved-{Topic}" labels for purposes like this. Easy to override, but at least there's a paper trail.

@mikekistler
Copy link
Member

Currently this is a manual process -- one of the API Stewards assigned as a reviewer adds the "APIStewardshipBoard-signedOff" label and that is the signal that the PR is okay to merge (from the POV of the stewardship board).

Issue #6184 should put the checks in place to block a merge until the "signedOff" label has been applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIView Central-EngSys This issue is owned by the Engineering System team.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants