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

Regen resources#3192

Closed
RikkiGibson wants to merge 4 commits intoAzure:masterfrom
RikkiGibson:RegenResources
Closed

Regen resources#3192
RikkiGibson wants to merge 4 commits intoAzure:masterfrom
RikkiGibson:RegenResources

Conversation

@RikkiGibson
Copy link
Member

Should contain changes authored by @mentat9 in Azure/azure-rest-api-specs#3032

@RikkiGibson RikkiGibson requested a review from a user July 18, 2018 17:17
@azuresdkci
Copy link

Can one of the admins verify this patch?

"Srinivasan, Vivek <visriniv@microsoft.com>"
],
"version": "3.1.1-preview",
"version": "3.2.1-preview",
Copy link

Choose a reason for hiding this comment

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

Since the API version changed, please do a major version bump. I also think we can remove the -preview suffix.
Also, did you update the version number in the azure-rest-api-specs repository too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Major version bump seems good--haven't changed anything in rest-api-specs. The automation there is also broken for mystery reasons which is why I manually opened this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Major version bump seems good--haven't changed anything in rest-api-specs. The automation there is also broken for mystery reasons which is why I manually opened this PR.

The changes in the policy SDK are pretty significant. I don't know how versioning is done for this SDK, but my REST changes were to add a new [non-preview] API version for policy, so that should be reflected here using whatever versioning scheme is appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, I'll go with 4.0.0

@mentat9
Copy link
Member

mentat9 commented Jul 18, 2018

Should contain changes authored by @mentat9 in Azure/azure-rest-api-specs#3032

Looks like my stuff is in there. Looks reasonable, at least to this non-Node.js guy. Thanks for getting these changes in!

@RikkiGibson
Copy link
Member Author

I think some Nock tests might need to be rerecorded.

@ghost
Copy link

ghost commented Oct 4, 2018

We've released several times since this PR (current azure-arm-compute version is 8.2.0), so this PR isn't needed anymore. Any changes that this PR was trying to generate have already been generated and published.
@RikkiGibson I recommend closing (without merging) this PR.

@ghost ghost self-assigned this Oct 4, 2018
@ghost ghost added this to the Sprint-125 milestone Oct 4, 2018
@mentat9
Copy link
Member

mentat9 commented Oct 4, 2018

We've released several times since this PR (current azure-arm-compute version is 8.2.0), so this PR isn't needed anymore. Any changes that this PR was trying to generate have already been generated and published.
@RikkiGibson I recommend closing (without merging) this PR.

@RikkiGibson: I've responded in direct email: please don't close this PR until that thread is resolved.

@ghost
Copy link

ghost commented Oct 4, 2018

@mentat9 I've responded to your e-mail. If that clears things up, let me know if I can close this PR.

@ghost
Copy link

ghost commented Oct 8, 2018

@mentat9 I noticed that you closed your most recent azure-rest-api-specs PR. Does that mean that I can close this PR too?

@mentat9
Copy link
Member

mentat9 commented Oct 8, 2018

I believe we already confirmed that this change was no longer needed, because it was superseded a subsequent SDK regeneration (I never quite determined how it happened). If you think the current Node.js SDK reflects what is in the current policy swagger then, yes we can close this PR. As far as I could tell, it does.

@ghost ghost closed this Oct 8, 2018
@ghost ghost removed the in progress label Oct 8, 2018
This pull request was closed.
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.

3 participants

Comments