-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Implement secure key release #16794
Implement secure key release #16794
Conversation
Resolves Azure#14892 sans tests; see Azure#16789 and Azure#16792
FWIW, I did write some tests that failed because the service hasn't implemented it as spec'd in the swagger, but I did examine the test recordings and request/response models were correct as spec'd. @herveyw-msft do you want us to go ahead and check this in, then? /cc @vcolin7 |
I spoke with Vick offline and, yes, we will merge this. It may not work for a while, but should eventually. |
@vcolin7 could you at least review https://github.com/Azure/azure-sdk-for-net/pull/16794/files#diff-a5a8d91de8e78a2e9da740239ab1cf5c214ad8e30567fe1820465113c9b425fa and make sure that looks like what you expected given our conversations? |
sdk/keyvault/Azure.Security.KeyVault.Keys/api/Azure.Security.KeyVault.Keys.netstandard2.0.cs
Show resolved
Hide resolved
sdk/keyvault/Azure.Security.KeyVault.Keys/api/Azure.Security.KeyVault.Keys.netstandard2.0.cs
Show resolved
Hide resolved
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.
LGTM!
* Doc comment and scope name improvements * Implement secure key release Resolves Azure#14892 sans tests; see Azure#16789 and Azure#16792 * Update public APIs
Resolves #14892 sans tests; see #16789 and #16792