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 wp i18n make-php command #363

Merged
merged 12 commits into from
Jan 30, 2024
Merged

Add wp i18n make-php command #363

merged 12 commits into from
Jan 30, 2024

Conversation

swissspidy
Copy link
Member

@swissspidy swissspidy commented May 5, 2023

The WordPress core performance team is working on the Performant Translations feature project, which aims to speed up translations by using PHP files instead of MO files. The goal is to get it ready for WordPress 6.5.

While the idea is to have translate.wordpress.org ship these PHP files in language packs, some developers might be using WP-CLI for their i18n workflow, especially locally. For them it would be beneficial to be able to quickly generate the necessary translation files.

Additional Context

@swissspidy swissspidy changed the title Add make-php command Oct 3, 2023
@swissspidy swissspidy marked this pull request as ready for review November 9, 2023 12:50
@swissspidy swissspidy requested a review from a team as a code owner November 9, 2023 12:50
@danielbachhuber
Copy link
Member

@swissspidy What sort of review would you like on this?

src/MakePhpCommand.php Outdated Show resolved Hide resolved
@swissspidy
Copy link
Member Author

@danielbachhuber just a general review if the code and code style look sane. Functionality-/logic-wise I will be syncing this with any developments in core (see https://make.wordpress.org/core/2023/11/08/merging-performant-translations-into-core/ for the latest post)

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Command code generally seems fine. I didn't test the actual functionality, though. I left a few nits inline.

src/MakePhpCommand.php Show resolved Hide resolved
src/MakePhpCommand.php Outdated Show resolved Hide resolved
src/MakePhpCommand.php Outdated Show resolved Hide resolved
src/MakePhpCommand.php Outdated Show resolved Hide resolved
src/MakePhpCommand.php Show resolved Hide resolved
And the foo-plugin/foo-plugin-de_DE.php file should contain:
"""
'messages'=>[''=>['Foo Plugin'=>['Bar Plugin']]]
"""
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a test that verifies a valid PHP translation file is produced?

Copy link
Member Author

Choose a reason for hiding this comment

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

Valid as in syntax or as in translation gets loaded properly in WordPress?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking the latter but we could check both, I suppose.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is probably best done in a future PR because we don't have similar assertions for the other file formats, and the PHP format especially is not yet merged into core.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good 👍

@swissspidy swissspidy merged commit 763144a into main Jan 30, 2024
65 checks passed
@swissspidy swissspidy deleted the add/try-make-php branch January 30, 2024 09:27
@swissspidy swissspidy added this to the 2.5.1 milestone Jan 30, 2024
swissspidy added a commit that referenced this pull request Feb 1, 2024
Follow-up to #363. Requirement for WP 6.5 support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants