Skip to content

Correct Azure Key Vault Certificates .md files content inconsistency and unclear#10118

Merged
vcolin7 merged 27 commits intoAzure:masterfrom
FredGao-new:azure-keyvault-certificates
May 6, 2020
Merged

Correct Azure Key Vault Certificates .md files content inconsistency and unclear#10118
vcolin7 merged 27 commits intoAzure:masterfrom
FredGao-new:azure-keyvault-certificates

Conversation

@FredGao-new
Copy link
Contributor

Create PR to address this issue.

@tzhanl tzhanl requested a review from jongio April 13, 2020 10:33
@@ -69,7 +69,7 @@ Here is [Azure Cloud Shell](https://shell.azure.com/bash) snippet below to
* Grant the above mentioned application authorization to perform key operations on the keyvault:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to perform certificate operations?

Create a Certificate to be stored in the Azure Key Vault.
- `beginCreateCertificate` creates a new certificate in the key vault. if the certificate with name already exists then a new version of the certificate is created.
Create a certificate to be stored in the Azure Key Vault.
- `beginCreateCertificate` creates a new certificate in the Azure Key Vault. if the certificate with name already exists then a new version of the certificate is created.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. if --> . If

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all certificate be updated to Certificate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be the other way around.

// The List Certificates operation returns certificates without their full properties, so for each certificate returned we call `getCertificate` to get all its attributes excluding the policy.
certificateAsyncClient.listPropertiesOfCertificates()
.subscribe(certificateProperties -> certificateAsyncClient.getCertificate(certificateProperties.getName(),
.subscribe(certificateProperties -> certificateAsyncClient.getCertificateVersion(certificateProperties.getName(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line update to Key Vault.

Copy link
Member

@vcolin7 vcolin7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a number of change requests. Also, there are many other small improvements and corrections I think we can make to this file but couldn't add them to this review, is it possible I can contribute to your branch so I can add them @FredGao-new?

FredGao-new and others added 19 commits April 27, 2020 14:29
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
Co-Authored-By: vcolin7 <vicolina@microsoft.com>
@vcolin7
Copy link
Member

vcolin7 commented Apr 29, 2020

/check-enforcer reset

@vcolin7
Copy link
Member

vcolin7 commented Apr 29, 2020

/check-enforcer evaluate

2 similar comments
@mitchdenny
Copy link
Contributor

/check-enforcer evaluate

@vcolin7
Copy link
Member

vcolin7 commented Apr 29, 2020

/check-enforcer evaluate

@vcolin7
Copy link
Member

vcolin7 commented Apr 29, 2020

/azp run java - keyvault - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

System.out.println("Delete Status: " + pollResponse.getStatus().toString());
System.out.println("Delete Certificate Name: " + pollResponse.getValue().getName());
System.out.println("Certificate Delete Date: " + pollResponse.getValue().getDeletedOn().toString());
System.out.printf("Deletetion status: %s\n", pollResponse.getStatus().toString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo , should be Deletion.

@vcolin7
Copy link
Member

vcolin7 commented May 6, 2020

/check-enforcer reset

@vcolin7
Copy link
Member

vcolin7 commented May 6, 2020

/check-enforcer evaluate

@vcolin7
Copy link
Member

vcolin7 commented May 6, 2020

/check-enforcer reset

@vcolin7
Copy link
Member

vcolin7 commented May 6, 2020

/check-enforcer evaluate

@vcolin7 vcolin7 merged commit a2e36dd into Azure:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants