Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Update AzureKeyVaultVersion to 2.3.2 #692

Merged
merged 1 commit into from
Jul 11, 2017
Merged

Update AzureKeyVaultVersion to 2.3.2 #692

merged 1 commit into from
Jul 11, 2017

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Jul 11, 2017

@pakrym pakrym requested review from divega and JunTaoLuo July 11, 2017 16:23
divega
divega reviewed Jul 11, 2017
View reviewed changes
Copy link

@divega divega left a comment

Choose a reason for hiding this comment

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

So we did not have to make code changes for the breaking changes after all?

@pakrym
Copy link
Contributor Author

pakrym commented Jul 11, 2017

Doesn't seem to be compile time breaking change, looking right now what exactly changed.

@pakrym
Copy link
Contributor Author

pakrym commented Jul 11, 2017

It is strange https://github.com/Azure/azure-sdk-for-net/blob/3f736b5af3851ab99bbaa98483ae537de0d48cfb/src/SDKs/KeyVault/dataPlane/Microsoft.Azure.KeyVault/Customized/KeyVaultClient.cs#L41 method is there and was not changed in between 2.3.1 and 2.3.2

@divega
Copy link

divega commented Jul 11, 2017 •
edited
Loading

It is strange https://github.com/Azure/azure-sdk-for-net/blob/3f736b5af3851ab99bbaa98483ae537de0d48cfb/src/SDKs/KeyVault/dataPlane/Microsoft.Azure.KeyVault/Customized/KeyVaultClient.cs#L41 method is there and was not changed in between 2.3.1 and 2.3.2

@dragav @RandalliLama any insights on this? According to #569 (comment) there was a breaking change in this constructor.

divega
divega approved these changes Jul 11, 2017
View reviewed changes
Copy link

@divega divega left a comment

Choose a reason for hiding this comment

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

LGTM, assuming the still unresolved questions about the breaking change symptoms do not affect the outcome.

JunTaoLuo
JunTaoLuo approved these changes Jul 11, 2017
View reviewed changes
Copy link
Contributor

@JunTaoLuo JunTaoLuo left a comment

Choose a reason for hiding this comment

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

Approved when tested and travis/appveyor pass.

@pakrym pakrym merged commit ca5916a into rel/2.0.0 Jul 11, 2017
@JunTaoLuo JunTaoLuo deleted the pakrym/kv-232 branch July 11, 2017 23:14
@mikeharder mikeharder mentioned this pull request Jul 17, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@divega divega divega approved these changes

@JunTaoLuo JunTaoLuo JunTaoLuo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pakrym @divega @JunTaoLuo @dnfclas