Skip to content

Upgrade V6 SDK Generator Version for Event Grid#13361

Merged
sarangan12 merged 1 commit intoAzure:masterfrom
sarangan12:UpgradeV6SDKEventGrid
Jan 26, 2021
Merged

Upgrade V6 SDK Generator Version for Event Grid#13361
sarangan12 merged 1 commit intoAzure:masterfrom
sarangan12:UpgradeV6SDKEventGrid

Conversation

@sarangan12
Copy link
Contributor

The last V6 SDK version that is used to generate Event Grid is 6.0.0-dev.20200618.1. This PR is to upgrade the V6 SDK version to the latest 6.0.0-dev.20210121.2 version.

@ellismg Please review and approve.

@joheredi @ramya-rao-a FYI.......

readonly endpointUrl: string;
sendCloudEvents(events: SendCloudEventInput<any>[], options?: SendCloudEventsOptions): Promise<SendEventsResponse>;
sendCustomSchemaEvents(events: Record<string, any>[], options?: SendCustomSchemaEventsOptions): Promise<SendEventsResponse>;
// Warning: (ae-forgotten-export) The symbol "SendEventsResponse" needs to be exported by the entry point index.d.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line of code change is not really related to Version Upgrade. I just found a forgotten export and added it. Please let me know if this is not exported intentionally. I can revert the changes. Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Nope, this was just me forgetting to export something. Thank you for fixing!


// @public
export interface StorageLifecyclePolicyCompletedEventData {
// Warning: (ae-forgotten-export) The symbol "StorageLifecyclePolicyActionSummaryDetail" needs to be exported by the entry point index.d.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line of code change is not really related to Version Upgrade. I just found a forgotten export and added it. Please let me know if this is not exported intentionally. I can revert the changes. Thanks

/**
* The event data for a Job output asset.
*/
export type MediaJobOutputAsset = MediaJobOutput & {
Copy link
Member

Choose a reason for hiding this comment

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

Were there changes in the SWAGGER? I see this type has a new property odataType (below). Also saw a couple of other definitions with new properties

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. These changes are due to the generator. In old generator, when we have the odata type only the base class has odata type defined (possibly with a union type). With the new generator, even the subclasses have their own odata types and the base class has its own odata type (mostly not used at all)

etag?: string;
key?: string;
label?: string;
syncToken?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have this new property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was added with this commit: Azure/azure-rest-api-specs#11819 2 months back. The swagger is pointing to master branch. So, it is picking up new changes. The changes seemed minimal. So, I took them it

Copy link
Member

Choose a reason for hiding this comment

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

Okay, this is not a breaking change since the new properties are optional, however I would like @ellismg to confirm if we want to start exposing these properties now. Other than this I think the changes look reasonable and all checks pass

Copy link
Member

Choose a reason for hiding this comment

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

Exposing these now is great!

Copy link
Member

@ellismg ellismg left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for upgrading the generator!

etag?: string;
key?: string;
label?: string;
syncToken?: string;
Copy link
Member

Choose a reason for hiding this comment

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

Exposing these now is great!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants