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

[Hub Generated] Review request for Microsoft.Devices to add version stable/2019-03-22 #6932

Merged

Conversation

reshantchandra
Copy link
Contributor

@reshantchandra reshantchandra commented Aug 12, 2019

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

Contribution checklist:

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Aug 12, 2019

In Testing, Please Ignore

[Logs] (Generated from f05eef4, Iteration 19)

Succeeded .NET: test-repo-billy/azure-sdk-for-net [Logs] [Diff]
Succeeded Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]
Failed Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]
Warning Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]
  • Warning iothub/mgmt/2016-02-03 [Logs]
  • Warning iothub/mgmt/2017-01-19 [Logs]
  • Warning iothub/mgmt/2017-07-01 [Logs]
  • Warning iothub/mgmt/2018-01-22 [Logs]
  • Warning iothub/mgmt/2018-04-01 [Logs]
  • Warning preview/iothub/mgmt/2018-12-01-preview [Logs]
  • Warning preview/iothub/mgmt/2019-03-22-preview [Logs]
Succeeded JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]
Succeeded Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]

@AutorestCI
Copy link

AutorestCI commented Aug 12, 2019

Automation for azure-sdk-for-python

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Aug 12, 2019

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@reshantchandra reshantchandra added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Aug 12, 2019
@jlian
Copy link
Member

jlian commented Aug 12, 2019

Reason why we closed the PR for now: just talked to @reshantchandra and we noticed that the locations property isn't being properly returned by IoT Hub service for this API version (unlike 2019-03-22-preview). We figured the best thing to do is to close the PR until we figure out what to do.

The part that's missing

    "locations": [
      {
        "location": "West US",
        "role": "primary"
      },
      {
        "location": "East US",
        "role": "secondary"
      }
    ],

@reshantchandra
Copy link
Contributor Author

We resolved the location property issue now we see primary and secondary location as expected.

reshantchandra and others added 6 commits August 20, 2019 15:16
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
Copy link
Member

@jlian jlian left a comment

Choose a reason for hiding this comment

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

Small description update

@reshantchandra reshantchandra removed the DoNotMerge <valid label in PR review process> use to hold merge after approval label Aug 21, 2019
reshantchandra and others added 2 commits August 20, 2019 23:30
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
…/2019-03-22/iothub.json

Co-Authored-By: John Lian <jlian@users.noreply.github.com>
@mmyyrroonn mmyyrroonn added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 22, 2019
@anthony-c-martin anthony-c-martin 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 Aug 27, 2019
@yungezz
Copy link
Member

yungezz commented Aug 28, 2019

hi @reshantchandra @myronfanqiu what's the status of this PR? is it ready for merge?

Copy link
Contributor

@mmyyrroonn mmyyrroonn left a comment

Choose a reason for hiding this comment

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

LGTM. Compared with the corresponding preview version. Just small change. Please solve the ARM's comment and I can merge this PR.

@mmyyrroonn
Copy link
Contributor

@reshantchandra Please resolve the ARM's comments. @jlian Looks like the author has updated the PR. Could you please check it and approve this PR?

@reshantchandra
Copy link
Contributor Author

comments resolved

@mmyyrroonn
Copy link
Contributor

@jlian If you think this PR is good, please approve it so that I can merge this PR.

@mmyyrroonn mmyyrroonn merged commit d90a116 into Azure:master Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants