-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Api and sdk updates for bicep client #44181
Conversation
Thank you for your contribution @anamikapan! We will review the pull request and get back to you soon. |
API change check APIView has identified API level changes in this PR and created following API reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your account lacks the required public GitHub organizations and permissions required of an internal contributor. Please review the Azure SDK onboarding documentation and use the associated Teams channel for support.
- Azure SDK onboarding (Microsoft internal)
- Azure SDK onboarding assistance (Microsoft internal)
You can verify the state of your account by running the Validate-AzsdkCodeOwner script from the Azure SDK tools repository.
Please also be sure to add yourself to CODEOWNERS for this library, if you will be maintaining it going forward.
@jsquire I have requested the specific permissions. Also, the build failures on the PR are because of some spell checks and words that are a part of swagger changes. I have no control over that. Can we manually try to override the spell check errors ? |
@anamikapan : You now have the correct permissions; however you still have not made your Azure and Microsoft org memberships public. Please use the script that I referenced above to validate your account after updating your settings. @ArthurMa1978 : Can you confirm whether or not your team will own this library going forward? |
You'll want to update the cspell config with exceptions specific to this library. Please follow the existing pattern in the file. |
@jsquire should cspell config additions be part of the same PR? or should that go in before this. can you please clarify |
No, the service team is responsible for managing the library, and we will assist with the release process. |
Thanks, Arthur. @anamikapan: Arthur's response indicates that you will also be required to provide CODEOWNERS data for this PR to move forward. |
Please include them as part of this PR. |
Account issues have been resolved. CODEOWNERS content is still required to move forward. |
@jsquire I'm one of the engineering managers for Azure Deployments. The ARM package (sdk/resources/Azure.ResourceManager.Resources/**) that is modified by @anamikapan in this PR is owned by multiple teams within the ARM organization. I'm perfectly happy to take ownership for the parts of the package that my team owns and add ourselves to the list but we cannot be responsible for the whole package. The parts that we can own are the paths related to the following resource types (and any nested resource types below them) in the
|
Thanks, @majastrz. There will need to be a CODEOWNERS entry added as part of this PR around L965 in the management package section which follows the existing format. Since a library is represented by a single label for issue support purposes, I'd suggest that you create a GitHub team and use that as the owner so that you can more easily manage adding/removing contacts from it. |
@jsquire We have an existing team (@Azure/bicep-admins) that covers Azure Deployments functionality. Can we specify multiple teams? |
I believe so. @JimSuplizio - can you confirm? |
sdk/resources/Azure.ResourceManager.Resources/src/Generated/Extensions/ResourcesExtensions.cs
Show resolved
Hide resolved
So long as the team is under the azure-sdk-write team it'll get expanded into the individual users. |
Requested changes are related to swagger - Azure/azure-rest-api-specs#28204