Azure / azure-sdk-tools

Tools repository leveraged by the Azure SDK team.
MIT License
113 stars 177 forks source link

Scheduling app should allow service teams to request an "offline review" #6171

Open mikekistler opened 1 year ago

mikekistler commented 1 year ago

Service teams often have small changes that don't require an API review meeting, or have time pressures where they need review/approval earlier than the earliest available API review meeting slot. In these cases, I instruct the service team to schedule the review for the earliest available date and then send a follow-up email requesting an "offline review". I think this process should be streamlined for this case and the app should make this option readily available for service teams.

maririos commented 1 year ago

There are 2 things here:

  1. Do people always contact you because they know you are part of the board? or is it documented somewhere and the documentation sends them to you? (cc @ladonnaq for process awerness)
  2. What specifically you would like to change in the Scheduler tool and what behavior should it take?

@mikekistler I assigned the issue to you until we have more clarity on the request

mikekistler commented 1 year ago

1) I think generally I get these requests when I am listed as a reviewer on the PR, but sometimes just because I am a board member. 2) I don't want to be too prescriptive here -- the solution should allow a service team to request an "offline" review, presumably earlier than the next available API review slot. One possible solution -- hopefully easy -- is to just add a checkbox on the scheduler page like "Offline review requested". When checked, the tool would email the review request to the "Core" review team, possibly in addition to scheduling an actual meeting. Another approach would be add a field "Approval needed by" (which seems like good information anyway) and we could use that schedule offline reviews as needed.

maririos commented 1 year ago

Other questions:

maririos commented 1 year ago

ARM team follows a similar approach where not every team needs a review, is that something the API stewardship is moving towards?

ladonnaq commented 1 year ago

ARM team follows a similar approach where not every team needs a review, is that something the API stewardship is moving towards?

image

image

mikekistler commented 1 year ago

Is an offline review the same as reviewing the PR?

Yes.

Are there specific cases where the option should be communicated with the user? or always have it there

I can't think of any situations to hide it.

Is the "core" review team a DL?

azureapirbcore@microsoft.com

It will be good to add that possibility to the documentation so people are aware of it offline the tool too: https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/665/Azure-REST-API-Stewardship-Review

👍

ladonnaq commented 1 year ago

Is an offline review the same as reviewing the PR?

Yes.

Are there specific cases where the option should be communicated with the user? or always have it there

I can't think of any situations to hide it.

Is the "core" review team a DL?

azureapirbcore@microsoft.com

It will be good to add that possibility to the documentation so people are aware of it offline the tool too: https://dev.azure.com/azure-sdk/internal/_wiki/wikis/internal.wiki/665/Azure-REST-API-Stewardship-Review

👍

Hello @mikekistler Not sure if you read my comment with the information regarding the ARM REST API Review process. The ARM guidance is very clear as to when "live" meeting is required vs electronic review via the "Swagger PR review process". Could you provide similar guidance for data plane?

mikekistler commented 1 year ago

@ladonnaq At this point I don't want to prevent any service team from requesting an offline review. The immediate problem to solve is that they have not ability to do so (outside of informal teams messages or emails). Let's get the capability enabled first. It is just a request -- we can say "No". If at some point saying "No" becomes burdensome we can put additional guidelines in place.

maririos commented 1 year ago

As part of moving documentation to https://eng.ms/docs/products/azure-developer-experience/design/api-review we should take advantage and add when an offline meeting is required vs no. For example for ARM: https://armwiki.azurewebsites.net/rp_onboarding/process/api_review.html#review-meeting-guidance

mikekistler commented 1 year ago

I continue to receive requests for offline reviews. Just today I have received two -- one from the Form Recognizer team and another from the ICS Auth service. Making special arrangements for each of these is time consuming. We really need this feature to be built into the app.

maririos commented 1 year ago

I have bumped the priority so we consider this issue when planning work