-
Notifications
You must be signed in to change notification settings - Fork 21
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
Change OAuth token input name #308
Change OAuth token input name #308
Conversation
…ganizationsMembersOAuthProvidersProviderInformationRequest
* Request type for `organizations.members.oauthProviders.google`, | ||
* `organizations.members.oauthProviders.microsoft`. | ||
*/ | ||
export interface B2BOrganizationsMembersOAuthProvidersMicrosoftRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted, it seems we could keep this interface around (or have it trivially defined as equivalent to B2BOrganizationsMembersOAuthProvidersProviderInformationRequest
) and mark it deprecated. Then it wouldn't be a breaking change, which would (a) give consumers who are affected time to update before we "break" them, and (b) avoid a breaking change for anyone not affected (including anyone not using TS in the first place).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is reasonable. We just need a manual TYPES section with this definition (kind of like here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'll try to do this for all of the backend SDKs
7cb1039
to
5c7a6cb
Compare
@@ -80,8 +80,8 @@ export type { | |||
|
|||
export type { | |||
B2BOrganizationsMembersOAuthProvidersGoogleResponse, | |||
B2BOrganizationsMembersOAuthProvidersMicrosoftRequest, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is this generated too? It's not exporting the deprecated type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is generated since I don't recall ever having to manually edit it - @logan-stytch do you know why it might not be exporting the type here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, there's a section for exporting the manual types. I'll add it
|
||
// MANUAL(ProviderInformationRequest)(TYPES) | ||
/** | ||
* @deprecated Since version 10.11.0. Please use B2BOrganizationsMembersOAuthProvidersProviderInformationRequest instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny nit
* @deprecated Since version 10.11.0. Please use B2BOrganizationsMembersOAuthProvidersProviderInformationRequest instead. | |
* @deprecated Since version 10.11.0. Please use {@link B2BOrganizationsMembersOAuthProvidersProviderInformationRequest} instead. |
We're renaming
B2BOrganizationsMembersOAuthProvidersMicrosoftRequest
toB2BOrganizationsMembersOAuthProvidersProviderInformationRequest
, since it's used for more than just Microsoft OAuth. The old type is still left in to avoid a breaking change, but marked as deprecated.