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

HybridDataManager - Adding SDK version 1.0.1 to correspond to API version 2019-06-01 #10531

Merged
merged 5 commits into from
Apr 7, 2020

Conversation

prtank
Copy link
Contributor

@prtank prtank commented Mar 11, 2020

No description provided.

@prtank
Copy link
Contributor Author

prtank commented Mar 11, 2020

PR for spec merge of API version 2019-06-01 is Azure/azure-rest-api-specs#8676

@isra-fel isra-fel self-assigned this Mar 12, 2020
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

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

Hi @prtank generally the API spec needs to be merged into master BEFORE you generate SDK and submit this PR.

I noticed that the PR is missing a change to /eng/mgmt/mgmtmetadata/hybriddatamanager_resource-manager.txt, also the license headers are difference. I suspect it's caused by running autorest directly, rather that our recommended way: running src/generate.ps1 in your module.

So, please do the following things after the swagger review PR is merged:

  1. Edit src/generate.ps1, change -AutoRestVersion "latest" to "2.0.4413"
  2. Run it to generate SDK
    1. In case you encounter any error, please refer to our document: https://github.com/Azure/adx-documentation-pr/blob/master/engineering/adx_netsdk_process.md#generating-net-sdk
  3. Make sure you check in the txt file mentioned above

Thank you

@isra-fel isra-fel assigned prtank and unassigned isra-fel Mar 13, 2020
@isra-fel isra-fel added the Mgmt This issue is related to a management-plane library. label Mar 16, 2020
@isra-fel
Copy link
Member

kindly ping @prtank

@prtank
Copy link
Contributor Author

prtank commented Apr 3, 2020

Hi,

I have been waiting for the swagger spec PR to be merged before I take care of these changes. I am expecting that to go through today so I will take care of all these comments before Monday.

Thanks for reviewing in advance :)

@prtank
Copy link
Contributor Author

prtank commented Apr 3, 2020

Hi @prtank generally the API spec needs to be merged into master BEFORE you generate SDK and submit this PR.

I noticed that the PR is missing a change to /eng/mgmt/mgmtmetadata/hybriddatamanager_resource-manager.txt, also the license headers are difference. I suspect it's caused by running autorest directly, rather that our recommended way: running src/generate.ps1 in your module.

So, please do the following things after the swagger review PR is merged:

  1. Edit src/generate.ps1, change -AutoRestVersion "latest" to "2.0.4413"

  2. Run it to generate SDK

    1. In case you encounter any error, please refer to our document: https://github.com/Azure/adx-documentation-pr/blob/master/engineering/adx_netsdk_process.md#generating-net-sdk
  3. Make sure you check in the txt file mentioned above

Thank you

I have pushed the changed for AutoRest version 2.0.4413, but am curious as to why we are using that specific version instead of the latest - 3.0.6187

@prtank prtank requested a review from isra-fel April 5, 2020 02:39
@isra-fel isra-fel self-assigned this Apr 7, 2020
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

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

LGTM!

@isra-fel isra-fel merged commit 427fe55 into Azure:master Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants