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.

Sign up for GitHub

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

Jump to bottom

Fix missing type in netapp #8170

Merged
merged 1 commit into from
Jan 21, 2020
Merged

Fix missing type in netapp #8170

merged 1 commit into from
Jan 21, 2020

Conversation

ArcturusZhang
Copy link
Member

This PR fixes a missing type indicator in all api-versions of netapp service.
This missing type indicator will cause type of the particular field falls back to the base type of every all types in go SDK (aka interface{}) which is not very user friendly.

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@PhoenixHe-NV
Copy link
Contributor

/azp run automation - sdk

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jan 16, 2020 •
edited
Loading

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jan 16, 2020 •
edited
Loading

azure-sdk-for-java - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jan 16, 2020 •
edited
Loading

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from e58d11a with merge commit 0d9ed0e. SDK Automation 13.0.17.20191226.1
  • ️✔️@azure/arm-netapp [Logs]  [Release SDK Changes] 
    [npmPack] npm WARN deprecated rollup-plugin-node-resolve@5.2.0: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-node-resolve.
[npmPack] loaded rollup.config.js with warnings
[npmPack] (!) Unused external imports
[npmPack] default imported from external module 'rollup' but never used
[npmPack] 
[npmPack] ./esm/azureNetAppFilesManagementClient.js → ./dist/arm-netapp.js...
[npmPack] created ./dist/arm-netapp.js in 373ms

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jan 16, 2020 •
edited
Loading

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jan 16, 2020 •
edited
Loading

azure-sdk-for-python - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from e58d11a with merge commit 0d9ed0e. SDK Automation 13.0.17.20191226.1
  • ️✔️azure-mgmt-netapp [Logs]  [Release SDK Changes] 
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)
[build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)

This was referenced Jan 16, 2020
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@akning-ms
Copy link
Contributor

@ArcturusZhang Can I know what issue is this PR fix?

@ArcturusZhang
Copy link
Member Author

@ArcturusZhang Dapeng Zhang FTE Can I know what issue is this PR fix?

Actually this will not fix any issue, this is a refine.
For detail, the current version of autorest (which is using in the SDK automation v2) will ignore this absence of type and fallback to the base type for this field. But a newer version of autorest will not tolerant this absence of type, therefore will cause issues then.
Also, this fix will make the current type in SDK more user friendly.

@akning-ms akning-ms merged commit 0d9ed0e into Azure:master Jan 21, 2020
@ArcturusZhang ArcturusZhang deleted the fix-type-in-netapp branch January 22, 2020 04:29
@audunn audunn mentioned this pull request Mar 20, 2020
Merged
5 tasks
@akning-ms akning-ms added the LintRuleCandidate Lint Rule Candidate label Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@akning-ms akning-ms

Labels
LintRuleCandidate Lint Rule Candidate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ArcturusZhang @PhoenixHe-NV @akning-ms