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

[license] Clean-up terminology to match codebase #14531

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Sep 7, 2024

Those are the terminology that we use in the other parts of the codebase (the other repositories). Normally, this has no public API implications.

@oliviertassinari oliviertassinari added core Infrastructure work going on behind the scenes package: x-license Specific to @mui/x-license. labels Sep 7, 2024
@mui-bot
Copy link

mui-bot commented Sep 7, 2024

Deploy preview: https://deploy-preview-14531--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 0ead15a

Copy link
Member

@flaviendelangle flaviendelangle left a 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];
Copy link
Member

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];
Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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.

@oliviertassinari oliviertassinari enabled auto-merge (squash) September 11, 2024 17:19
@oliviertassinari oliviertassinari merged commit a246244 into mui:master Sep 11, 2024
18 checks passed
@oliviertassinari oliviertassinari deleted the license-remove-deprecated-API branch September 11, 2024 17:19
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 11, 2024

Next step:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes package: x-license Specific to @mui/x-license.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants