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

[Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 #5139

Merged
merged 9 commits into from
Feb 5, 2019
Merged

[Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 #5139

merged 9 commits into from
Feb 5, 2019

Conversation

shbha
Copy link
Contributor

@shbha shbha commented Feb 1, 2019 •
edited
Loading

If you are a MSFT employee you can view your work branch via this link.

Contribution checklist:

@AutorestCI AutorestCI mentioned this pull request Feb 1, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-js

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-js#693

@AutorestCI AutorestCI mentioned this pull request Feb 1, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#2158

@AutorestCI AutorestCI mentioned this pull request Feb 1, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-ruby

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-ruby#2207

@AutorestCI AutorestCI mentioned this pull request Feb 1, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#4033

@shbha
Copy link
Contributor Author

shbha commented Feb 1, 2019

@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-java

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-java#2917

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI AutorestCI mentioned this pull request Feb 1, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Feb 1, 2019 •
edited
Loading

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#4306

@veronicagg veronicagg assigned veronicagg and unassigned anuchandy Feb 1, 2019
@veronicagg veronicagg requested review from veronicagg and removed request for anuchandy February 1, 2019 21:13
@veronicagg
Copy link
Contributor

Taking this PR as I was reviewing the original, which is getting abandoned.

@veronicagg veronicagg mentioned this pull request Feb 1, 2019
Closed
5 tasks
@ravbhatnagar ravbhatnagar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 1, 2019
@ravbhatnagar
Copy link
Contributor

Doesnt look like any changes other than api version change. Signing off from ARM

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 1, 2019
veronicagg
veronicagg suggested changes Feb 1, 2019
View reviewed changes
Copy link
Contributor

@veronicagg veronicagg left a comment

Choose a reason for hiding this comment

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

Just one comment to update Java code generation in reamde.md file, rest looks good! Thanks for submitting a new PR with clearer commits.

specification/consumption/resource-manager/readme.md Outdated Show resolved Hide resolved
@veronicagg
Copy link
Contributor

@anuchandy could you help taking a look at the failure for Java generation? Java was missing from swagger to SDK configuration and as I added it, I'm seeing it failing. Thanks!

@veronicagg
Copy link
Contributor

@shbha I noticed Java SDK wasn't getting generated, I added the language to the readme.md configuration and it's now producing a failure, so would like to hear back from one of the Java SDK owners. After solving that failure, we should be good to merge this PR.

@shbha
Copy link
Contributor Author

shbha commented Feb 4, 2019

@veronicagg - Delay merge or PR. Upon internal review of this PR there is a requirement to introduce additional changes. 1) define and use method.parameter scope and introduce a few new scopes for existing methods. Arm provided this review on a different PR ( for a different specification RP) and I would like to follow best practices here as well. I would really appreciate it if we could hold off on merge till these changes are made.

@veronicagg veronicagg changed the title [Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 [Do not merge] [Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 Feb 4, 2019
@veronicagg
Copy link
Contributor

@shbha sure, thanks for letting me know, I've edited the title to indicate this. In the meantime, we're investigating the Java SDK failure generation. Please keep me posted on progress, thanks!

@shbha1
introducing MG scope for tags/budgets
ac1bedd 
introduce scope (method) parameter for APIs that support numerous scopes (arm-team suggestion implemented)
@shbha shbha changed the title [Do not merge] [Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 [Hub Generated] Review request for Microsoft.Consumption to add version 2019-01-01 Feb 4, 2019
@shbha
Copy link
Contributor Author

shbha commented Feb 4, 2019

The required changes have been made @veronicagg. Thank you

@anuchandy
Copy link
Member

@shbha could you please merge this PR shbha#1? This fixes Java code generation for previous api-version.

@anuchandy @shbhamsft
Fixing Java code-generation (#1)
12bbe0d 
* Fixing Java code-generation

* Removing duplicate directive from java section
@AutorestCI AutorestCI mentioned this pull request Feb 5, 2019
Merged
veronicagg
veronicagg approved these changes Feb 5, 2019
View reviewed changes
Copy link
Contributor

@veronicagg veronicagg left a comment

Choose a reason for hiding this comment

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

Thanks for the updates.

@veronicagg veronicagg merged commit c9016f5 into Azure:master Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veronicagg veronicagg veronicagg approved these changes

Assignees

@veronicagg veronicagg

Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@shbha @AutorestCI @azuresdkci @veronicagg @ravbhatnagar @anuchandy @shbhamsft @shbha1