Skip to content
This repository was archived by the owner on May 5, 2023. It is now read-only.

Conversation

@AutorestCI
Copy link
Contributor

Created to accumulate context: keyvault/resource-manager

Add node config to keyvault for Swagger to SDK
@amarzavery
Copy link
Contributor

amarzavery commented Mar 14, 2018

@schaabs - This is an automatic one that got created due to the node.js readme in the rest-api-specs repo.
I saw that a PR was sent yesterday for KV #2456. We merged that PR into the keyvault branch. This one is automatic and defaults to master. @lmazuel told me that you are aware about this process and you have been doing something for KV python sdk. Now that we have this do we need a key vault branch?

/cc @lusitanian

@schaabs
Copy link
Contributor

schaabs commented Mar 14, 2018

Closing this pull request as we already have merged these changes in to the keyvault branch as stated above. We will eventually merge these into master from the keyvault branch.

@amarzavery @lmazuel This functionality seems to be a bit different from what I've seen in the python sdk repo. The change in azure-rest-api-specs repo was merged into the keyvault_preview branch. In the python sdk this results in a PR being submitted for an analogous branch in the sdk repo restapi_auto_keyvault_preview rather than into master. In that case I was able to rebase that branch to the branch where we are developing our preview. Is this the eventual expected flow for the node repo as well or has the flow changed?

@RikkiGibson
Copy link
Member

Could it be related to the fact that I opened a different PR for the keyvault config that was based on azure-rest-api-specs master branch?

In a way it seems weird for the base branch on azure-rest-api-specs to affect the base branch on the auto-PRs for the SDK repos, but also seems like it could be useful.

@RikkiGibson
Copy link
Member

BTW I pulled this down and regenerated with gulp codegen and it generated just the same, so I think we're on the right track 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants