-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[license] Clean-up terminology to match codebase #14531
[license] Clean-up terminology to match codebase #14531
Conversation
b70bc79
to
56f7276
Compare
Deploy preview: https://deploy-preview-14531--material-ui-x.netlify.app/ |
56f7276
to
9bba507
Compare
9bba507
to
0ead15a
Compare
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.
The renaming make a lot of sense to me
Just want to check how we handle the breaking change since 2 of those variables are exported
export const PLAN_VERSIONS = ['initial', 'Q3-2024'] as const; | ||
|
||
export type LicenseScope = (typeof LICENSE_SCOPES)[number]; | ||
export type PlanScope = (typeof PLAN_SCOPES)[number]; |
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.
This is public API strictly speaking
We can either keep exporting an LIcenseScope
next to PlanScope
, or don't care because nobody would actually use this variable
*/ | ||
'subscription', | ||
] as const; | ||
|
||
export type LicensingModel = (typeof LICENSING_MODELS)[number]; | ||
export type LicenseModel = (typeof LICENSE_MODELS)[number]; |
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.
This is public API strictly speaking
We can either keep exporting an LicensingModel
next to LicenseModel
, or don't care because nobody would actually use this variable
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.
We indeed use for this tool: https://tools-private.mui.com/prod/pages/x_licenseKey. But it's simple to handle on our side.
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.
But do we agree that it's a breaking change? Since it's exported from a package people install, and without an unstable
or equivalent prefix.
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.
Technically, it's a breaking change, but we have never documented it and there is no practical use case for developers, so I think it's ok.
We have #9346 about the root problem.
Next step:
|
Those are the terminology that we use in the other parts of the codebase (the other repositories). Normally, this has no public API implications.