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

core(i18n): delete i18n.createMessageInstanceIdFn #14251

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

alexnj
Copy link
Member

@alexnj alexnj commented Aug 1, 2022

Deprecate the legacy i18n.createMessageInstanceIdFn method in favor of the new createIcuMessageFn introduced in #10630.

If we have external dependencies that rely on this method, they will have to be updated.

Fixes #13993.

Copy link
Member

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

Change looks good to me for our code! This has to be breaking pubads, though. I assume that won't show up in our tests until #14241 lands reenables the pubads tests.

We should fix it over there too, and it should also be just a text replacement since the new function has been shipping for so long, but I don't know if we want to deal with branches and release schedules over there too (on top of the ESM stuff), so maybe it is better to leave the old method in for now with an @deprecated tag.

@connorjclark
Copy link
Collaborator

connorjclark commented Aug 2, 2022

Leaving it in for pubads sounds good to me. I'll take care of making the changes on that side.

Actually, I just opened a PR to fix it right now. Go ahead with a full removal here.

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

nit: rename title to "delete" (deprecation != removal)

@alexnj alexnj changed the title core(i18n): deprecate i18n.createMessageInstanceIdFn core(i18n): delete i18n.createMessageInstanceIdFn Aug 4, 2022
@alexnj alexnj merged commit d14c7e6 into GoogleChrome:master Aug 8, 2022
alexnj added a commit to alexnj/lighthouse that referenced this pull request Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove createMessageInstanceIdFn
4 participants