-
Notifications
You must be signed in to change notification settings - Fork 5
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
D7 upgrade path #59
D7 upgrade path #59
Conversation
…abs with fixed language
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.
@indigoxela I've added some comments. I made this PR from your branch so that I can comment on exact changes.
} | ||
// Hardcoded for now. | ||
// @see https://github.com/backdrop/backdrop-issues/issues/4888 | ||
$mail_translatables = array( |
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 can't predict when people will run this. I guess you're assuming this will be removed before core might add new translatable config to core and/or fix the core bug?
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 can't predict when people will run this...
If i18n_update_dependencies() isn't enough (it probably isn't), we could additionally use update_already_performed() here.
Actually I'm hoping that backdrop/backdrop#3504 gets merged into core soon enough, that I can revert this workaround. It's currently needed for proper testing - at least, I needed it to verify that this update hook works as expected.
Making a PR so I can comment on changes.