Skip to content
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

Add support and tests for SSL on custom domains in CDN #5406

Merged
merged 6 commits into from
Mar 19, 2019

Conversation

nachakra
Copy link
Contributor

@nachakra nachakra commented Mar 12, 2019

Link to swagger PR - Azure/azure-rest-api-specs#5276

Since we use an existing susbscription that is used for other testing too, the tests failed beacuse they assumed it is a clean subscription. I changed that assumption so that they(hopefully) work with any subscription in record mode.
Copy link
Member

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

You are missing Swagger PR information.
Paste a link to the swagger PR.
Also update nuget package version in .csproj, also update version in AssemblyInfo.cs
Generate SDK using generate.ps1, which will essentially generate all the meta data files that are required for .NET SDK

@nachakra
Copy link
Contributor Author

@shahabhijeet -

  1. I added the swagger link to the description.
  2. I updated the assemblyinfo.cs. When do we need to change the assemblyinfo.cs if it is not a breaking change. Also it is not a new version of API.
    .csproj is included in the PR and the nuget package version is already updated.
  3. I did use the generate.cmd to generate the SDK and I do see a metadata file in this PR - /SDKs/_metadata/cdn_resource-manager.txt
    Should there be more?

@nachakra
Copy link
Contributor Author

@shahabhijeet - can you take a look again at the PR please?

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

If you are planning to release this SDK, please update version info in .csproj and AssemblyInfo.cs
Also add release notes.

@nachakra
Copy link
Contributor Author

@shahabhijeet - I updated both in the last commit or the commits before. Not sure what I am still missing.

@shahabhijeet shahabhijeet merged commit 2fd9988 into Azure:master Mar 19, 2019
@nachakra nachakra deleted the cdnEnableSSLOnCustomDomains branch March 19, 2019 20:40
chidozieononiwu pushed a commit to chidozieononiwu/azure-sdk-for-net that referenced this pull request Mar 26, 2019
* Add support and tests for SSL on custom domains in CDN

* Fix the test failure for profiles

* Fix the build and refactor the profile tests

Since we use an existing susbscription that is used for other testing too, the tests failed beacuse they assumed it is a clean subscription. I changed that assumption so that they(hopefully) work with any subscription in record mode.

* Update nuget package version

* Update file version

* Update the moinor version since new functionality was added
mentat9 pushed a commit to mentat9/azure-sdk-for-net that referenced this pull request Jun 10, 2019
* Add support and tests for SSL on custom domains in CDN

* Fix the test failure for profiles

* Fix the build and refactor the profile tests

Since we use an existing susbscription that is used for other testing too, the tests failed beacuse they assumed it is a clean subscription. I changed that assumption so that they(hopefully) work with any subscription in record mode.

* Update nuget package version

* Update file version

* Update the moinor version since new functionality was added
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.

3 participants