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

fix: partial text domain update for the newspack plugin #2646

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

laurelfulford
Copy link
Contributor

@laurelfulford laurelfulford commented Sep 15, 2023

All Submissions:

Changes proposed in this Pull Request:

The text domain in the Newspack Plugin is incorrect -- it should be newspack-plugin, matching the plugin's slug, but it is newspack. Because of this, translations don't work.

This flew under the radar for a long time because the plugin just contained backend strings, but now that it has front-end facing text -- like RAS -- it's a lot more noticeable.

This PR fixes a handful of publisher-specific strings, to help with a launch and make this PR more reasonable to test. The plan is to handle the rest of the strings in small batches so we don't end up with overwhelming test scenarios.

See 1200550061930446-as-1205509524123559

How to test the changes in this Pull Request:

  1. Visually review the PR and make sure the only changes that have been made are changing newspack to newspack-plugin in the translation strings, and greatly reducing the POT file (only strings with the updated text-domain should show up there).
  2. Test to make sure translations actually load -- for this, can make your own .PO file and translate some of the strings and make sure they show up on the front-end. I also have a fake PO and MO German translation you can use - you can drop these two files in the /newspack-plugin/languages directory, change your site's language to German, and confirm some of the front-end RAS strings -- like the Sign In button, and Log Out link on My Account -- have 'translated' appended to them.

fake-german-translation.zip

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Sorry, something went wrong.

@laurelfulford laurelfulford added the [Status] Needs Review The issue or pull request needs to be reviewed label Sep 15, 2023
@laurelfulford laurelfulford requested a review from a team as a code owner September 15, 2023 16:30
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

Looks good!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Sep 15, 2023
@laurelfulford
Copy link
Contributor Author

Thanks @dkoo!

@laurelfulford laurelfulford merged commit 7d92092 into master Sep 15, 2023
@laurelfulford laurelfulford deleted the fix/update-text-domain-partial branch September 15, 2023 18:37
matticbot pushed a commit that referenced this pull request Sep 15, 2023
# [2.7.0-alpha.1](v2.6.1...v2.7.0-alpha.1) (2023-09-15)

### Bug Fixes

* partial text domain update for the newspack plugin ([#2646](#2646)) ([7d92092](7d92092))

### Features

* add ads.txt manager to plugin manager ([#2639](#2639)) ([06eccaa](06eccaa))
* **amp-deprecation:** remove AMP from supported list ([#2647](#2647)) ([ef98476](ef98476))
* disable deactivate and delete for Akismet ([#2593](#2593)) ([136752a](136752a))
* replace recommended cookie plugin ([#2223](#2223)) ([4a309be](4a309be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.7.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

laurelfulford added a commit that referenced this pull request Sep 20, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
matticbot pushed a commit that referenced this pull request Sep 20, 2023
## [2.6.2](v2.6.1...v2.6.2) (2023-09-20)

### Bug Fixes

* partial text domain update for the newspack plugin ([#2646](#2646)) ([#2654](#2654)) ([3b5b713](3b5b713))
matticbot pushed a commit that referenced this pull request Sep 20, 2023
# [2.7.0-alpha.2](v2.7.0-alpha.1...v2.7.0-alpha.2) (2023-09-20)

### Bug Fixes

* partial text domain update for the newspack plugin ([#2646](#2646)) ([#2654](#2654)) ([3b5b713](3b5b713))
matticbot pushed a commit that referenced this pull request Sep 25, 2023
# [2.7.0](v2.6.4...v2.7.0) (2023-09-25)

### Bug Fixes

* partial text domain update for the newspack plugin ([#2646](#2646)) ([7d92092](7d92092))

### Features

* add ads.txt manager to plugin manager ([#2639](#2639)) ([06eccaa](06eccaa))
* **amp-deprecation:** remove AMP from supported list ([#2647](#2647)) ([ef98476](ef98476))
* disable deactivate and delete for Akismet ([#2593](#2593)) ([136752a](136752a))
* replace recommended cookie plugin ([#2223](#2223)) ([4a309be](4a309be))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 2.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@dkoo dkoo mentioned this pull request Sep 29, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @alpha released [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants