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

add a warning on manual locale set #1812

Conversation

matthewmayer
Copy link
Contributor

@matthewmayer matthewmayer commented Feb 3, 2023

This is just a rough idea of how we could warn users who are setting locale at runtime but allow explicit import of a locale to work.

@matthewmayer matthewmayer added the do NOT merge yet Do not merge this PR into the target branch yet label Feb 3, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Feb 3, 2023

This is only applicable now.
Once #1735 is merged that no longer works.

@matthewmayer
Copy link
Contributor Author

This is only applicable now. Once #1735 is merged that no longer works.

Yes correct. My idea would be to land something like this PR in v8.0 so people have one major version to switch, and then land #1735 in v9.0

@ST-DDT
Copy link
Member

ST-DDT commented Feb 4, 2023

With #1735 we also add support for multiple fallback-locales, which IMO is a much needed feature. This also has an impact on #1441 . To simplify the migration for TS and JS users, we have the dummy/stub methods, that point to the new way (and in the future even to a chapter in the migration guide). If we schedule it to v9, then we delay the feature for roughly half a year. Please also read the v8 milestone descriptions (if you haven't already) to get a better idea of our planned roadmap.

Could you please list some pros/cons so we have a better understanding of your idea/motivation for using the longer migration path? We would like to see the whole picture before deciding on this. If you have any questions feel free to ask them in Discord.

One of our ideas for an explicit "deprecation" path is releasing a v7.7 that deprecates these methods, so it isn't just gone.
What do you think about that alternative?

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision deprecation A deprecation was made in the PR labels Feb 4, 2023
@matthewmayer
Copy link
Contributor Author

With #1735 we also add support for multiple fallback-locales, which IMO is a much needed feature. This also has an impact on #1441 . To simplify the migration for TS and JS users, we have the dummy/stub methods, that point to the new way (and in the future even to a chapter in the migration guide). If we schedule it to v9, then we delay the feature for roughly half a year. Please also read the v8 milestone descriptions (if you haven't already) to get a better idea of our planned roadmap.

Could you please list some pros/cons so we have a better understanding of your idea/motivation for using the longer migration path? We would like to see the whole picture before deciding on this. If you have any questions feel free to ask them in Discord.

One of our ideas for an explicit "deprecation" path is releasing a v7.7 that deprecates these methods, so it isn't just gone. What do you think about that alternative?

You don't necessarily have to wait 6 months between releasing v8 and v9. I'm a firm believer that semver-major releases don't have to be particularly "major", its just that they flag up breaking changes. In fact too many breaking changes in a release can put people off upgrading.

So basically I'm suggesting current v8.0 would be split into two,

@matthewmayer
Copy link
Contributor Author

If you don't want to do that, then a v7.7 containing just additional deprecation warnings to let people prepare for v8 is certainly better than nothing.

@ST-DDT
Copy link
Member

ST-DDT commented Feb 16, 2023

Team Decision

This proposal has been rejected.
If we would do that, then we would have to write a migration guide (to fix the warnings), but then the user is likely to downgrade to v7.6 because they cannot really use the new way or upgrade directly to v8.0. Also it is very likely, that there will be only a short period of time between versions, so it wouldn't help the users much.

Will will write a migration guide and explicitly highlight this change.

@ST-DDT ST-DDT closed this Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation A deprecation was made in the PR do NOT merge yet Do not merge this PR into the target branch yet p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants