Update the MediaServices.json file in the EventGrid DataPlane SDK #4111

quintinb · 2018-10-09T20:38:46Z

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

[ x ] The title of the PR is clear and informative.
[ x ] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
[ x ] Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
[ x ] If applicable, the PR references the bug/issue that it fixes.
[ x ] Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

[ x ] I have read the contribution guidelines.
[ x ] My spec meets the review criteria:
- [ x ] The spec conforms to the Swagger 2.0 specification.
- [ x ] The spec follows the guidelines described in the Swagger checklist document.
- [ x ] Validation tools were run on swagger spec(s) and have all been fixed in this PR.

…nitions

AutorestCI · 2018-10-09T20:39:33Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-10-09T20:40:38Z

Automation for azure-sdk-for-js

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

AutorestCI · 2018-10-09T20:41:46Z

Automation for azure-sdk-for-ruby

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-ruby#1734

AutorestCI · 2018-10-09T20:43:00Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-10-09T20:43:03Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

azuresdkci · 2018-10-09T20:59:55Z

Can one of the admins verify this patch?

kalyanaj · 2018-10-09T21:06:18Z

specification/eventgrid/data-plane/Microsoft.Media/stable/2018-01-01/MediaServices.json

        }
      }
+    },
+    "JobError": {


Will it be possible to prefix "Media" to such types (JobError, JobErrorDetail, JobOutput, etc.). Since autorest didn't fully support associating separate namespaces for each swagger in this set, all of these types will be under Microsoft.Azure.EventGrid.Models namespace, and hence having the Media prefix for your types will make it more easily discoverable and avoid confusion/conflicts.

kalyanaj · 2018-10-09T21:13:54Z

specification/eventgrid/data-plane/Microsoft.Media/stable/2018-01-01/MediaServices.json

+          "$ref": "#/definitions/JobOutput",
+          "description": "Gets the output."
+        },
+        "jobCorrelationData": {


Minor Nit: In MediaJobStateChangeEventData, a similar property is named "correlationData" while here it is "jobCorrelationData". Is this by design, or should these names be made consistent?

This is intentional because this is in the JobOutputStateChange related messages. The correlation data is a property of the Job object and not the JobOutput object (the Job contains the JobOutputs).

Thanks, that sounds good.

hovsepm · 2018-10-09T21:26:10Z

@quintinb please ping me directly when all the reviewers will sign of on the PR.

kalyanaj

I have added a couple of comments, the main one being the typenames.

kalyanaj

Thanks @quintinb for the changes. Will it be possible to add the Media prefix for the enum type (all instances of JobState, JobErrorCode, JobErrorCategory and JobRetry) names as well?

kalyanaj

Thanks @quintinb for the changes, the changes look good, I have added one additional comment/request regarding the names of the enum types as well, can you please consider that as well?

hovsepm · 2018-10-15T20:32:18Z

@quintinb could you take a look to @kalyanaj comment?

quintinb · 2018-10-16T20:35:02Z

Yes, I will look at adding the Media prefix to the enum names as well.

AutorestCI · 2018-10-16T21:13:11Z

Automation for azure-sdk-for-go

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

kalyanaj

LGTM.

quintinb added 4 commits October 1, 2018 09:33

Updated MediaServices.json to include the latest Media EventData defi…

5e033bd

…nitions

Fixing some internal references in MediaServices.json

1a756dd

Fixing errors identified by the AutoRest validator

79a3797

Making sure the polymorphic JobOutput is used instead of JobOutputAsset

1f765fd

AutorestCI mentioned this pull request Oct 9, 2018

[AutoPR eventgrid/data-plane] Update the MediaServices.json file in the EventGrid DataPlane SDK Azure/azure-sdk-for-python#3515

Merged

AutorestCI mentioned this pull request Oct 9, 2018

[AutoPR eventgrid/data-plane] Update the MediaServices.json file in the EventGrid DataPlane SDK Azure/azure-sdk-for-ruby#1733

Closed

AutorestCI mentioned this pull request Oct 9, 2018

[AutoPR eventgrid/data-plane] Update the MediaServices.json file in the EventGrid DataPlane SDK Azure/azure-sdk-for-node#3790

Merged

azuresdkci assigned hovsepm Oct 9, 2018

azuresdkci requested a review from hovsepm October 9, 2018 21:01

kalyanaj reviewed Oct 9, 2018

View reviewed changes

hovsepm approved these changes Oct 9, 2018

View reviewed changes

kalyanaj requested changes Oct 9, 2018

View reviewed changes

Updated definition names to all start with the Media prefix

549ec9e

kalyanaj reviewed Oct 10, 2018

View reviewed changes

kalyanaj requested changes Oct 10, 2018

View reviewed changes

Adding Media prefix to enum names

2b69660

This was referenced Oct 16, 2018

[AutoPR eventgrid/data-plane] Update the MediaServices.json file in the EventGrid DataPlane SDK Azure/azure-sdk-for-js#202

Merged

[AutoPR eventgrid/data-plane] Update the MediaServices.json file in the EventGrid DataPlane SDK Azure/azure-sdk-for-go#3038

Merged

kalyanaj approved these changes Oct 16, 2018

View reviewed changes

hovsepm merged commit 4c6fe46 into Azure:master Oct 16, 2018

quintinb mentioned this pull request Oct 17, 2018

Updating EventGrid DataPlane SDK with the new Events from Media Servi… Azure/azure-sdk-for-net#4900

Merged

Update the MediaServices.json file in the EventGrid DataPlane SDK #4111

Update the MediaServices.json file in the EventGrid DataPlane SDK #4111

Uh oh!

Conversation

quintinb commented Oct 9, 2018

PR information

Quality of Swagger

Uh oh!

AutorestCI commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-js

Uh oh!

AutorestCI commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-ruby

Uh oh!

AutorestCI commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-node

Uh oh!

AutorestCI commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-java

Uh oh!

azuresdkci commented Oct 9, 2018

Uh oh!

kalyanaj Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

kalyanaj Oct 9, 2018

Choose a reason for hiding this comment

Uh oh!

quintinb Oct 10, 2018

Choose a reason for hiding this comment

Uh oh!

kalyanaj Oct 11, 2018

Choose a reason for hiding this comment

Uh oh!

hovsepm commented Oct 9, 2018

Uh oh!

kalyanaj left a comment

Choose a reason for hiding this comment

Uh oh!

kalyanaj left a comment

Choose a reason for hiding this comment

Uh oh!

kalyanaj left a comment

Choose a reason for hiding this comment

Uh oh!

hovsepm commented Oct 15, 2018

Uh oh!

quintinb commented Oct 16, 2018

Uh oh!

AutorestCI commented Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

kalyanaj left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AutorestCI commented Oct 9, 2018 •

edited

Loading

AutorestCI commented Oct 9, 2018 •

edited

Loading

AutorestCI commented Oct 9, 2018 •

edited

Loading

AutorestCI commented Oct 9, 2018 •

edited

Loading

AutorestCI commented Oct 9, 2018 •

edited

Loading

AutorestCI commented Oct 16, 2018 •

edited

Loading