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

Update theme-i18n.json #366

Merged
merged 3 commits into from
Nov 4, 2023
Merged

Update theme-i18n.json #366

merged 3 commits into from
Nov 4, 2023

Conversation

swissspidy
Copy link
Member

@swissspidy swissspidy commented Jun 28, 2023

We really need a better way to keep this file in sync with core. Edit: forgot that we fetch directly from svn

@swissspidy swissspidy requested a review from a team as a code owner June 28, 2023 08:27
@swissspidy
Copy link
Member Author

cc @oandregal @gziolo if you could help me double check this

@oandregal
Copy link
Contributor

I've compared the changes with core and it looks good. I'm trying to find the tests instructions in older PRs.

@swissspidy swissspidy changed the title Update theme-i18n.json Update theme-i18n.json Jun 28, 2023
@oandregal
Copy link
Contributor

This is what I've done for testing (updated from #306):

  • Check out this PR locally and cd to the directory.
  • composer install
  • Update the THEME_JSON_SOURCE to be empty in this line at the JsonSchemaExtractor class, so the command uses the local backup file defined by THEME_JSON_FALLBACK.
  • Create a new directory called foo-theme that contains a theme.json file and paste the content of TwentyTwentyThree's theme.json.
  • Run vendor/bin/wp i18n make-pot foo-theme
  • Verify that there's a foo-theme/foo-theme.pot file and that strings and their context are present (look for "Custom template name", "Template part name", "Font family name", "Space size name", the ones added in this PR).

@oandregal
Copy link
Contributor

I've also run composer behat -- features/makepot.feature though the scenario 001 Scenario: Extract all supported functions failed. Same as in trunk, so unrelated to this PR.

@swissspidy swissspidy enabled auto-merge June 28, 2023 09:31
@swissspidy
Copy link
Member Author

Awesome, thanks for testing! 🎉

Totally forgot that this is just the backup file. That makes the change way less urgent 🙂

I'll see if I can reproduce that test failure locally.

@schlessera
Copy link
Member

schlessera commented Aug 31, 2023

We really need a better way to keep this file in sync with core.

We can add a GHA that replaces the included file with the one on SVN and creates a PR if changes have been detected.

@swissspidy
Copy link
Member Author

@schlessera Good idea. Just added such a workflow now.

@swissspidy swissspidy merged commit acfe4dd into main Nov 4, 2023
41 checks passed
@swissspidy swissspidy deleted the swissspidy-patch-1 branch November 4, 2023 11:50
@danielbachhuber danielbachhuber added this to the 2.4.5 milestone Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants