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.

Sign up for GitHub

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

Jump to bottom

feat(next): l10n: Update payments-next logic to use payments-next.ftl #18208

Merged
merged 1 commit into from
Jan 14, 2025
Merged

feat(next): l10n: Update payments-next logic to use payments-next.ftl #18208

merged 1 commit into from
Jan 14, 2025

Conversation

david1alvarez
Copy link
Contributor

Because

  • payments-next no longer needs to rely on payments translations, as payments-next translations are completed.

This pull request

  • Removes the dependency on payments.ftl translations within payments-next project.json and gruntfile.js

Issue that this pull request solves

Closes: #FXA-7841

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

@david1alvarez david1alvarez requested a review from a team as a code owner January 10, 2025 22:37
david1alvarez
david1alvarez commented Jan 10, 2025
View reviewed changes
apps/payments/next/Gruntfile.js
@@ -26,10 +26,6 @@ module.exports = function(grunt) {
src: srcPaths,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

its hard for me to tell if we still need this concat statement. It looks from reading the source code like it's still of use to us.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup it's still needed.

This grunt script creates the base payments-next.ftl file for base locale en which gets extracted by the l10n teams string extraction script, and passed on to Pontoon.

Additionally it also keeps the en/payments-next.ftl up to date during dev when new strings are added to the app.

StaberindeZA
StaberindeZA approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@StaberindeZA StaberindeZA left a comment

Choose a reason for hiding this comment

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

r+wc.

Could you please also clean up the remaining temporary scripts that were created? app/_lib/scripts/create-payments-next.ts and apps/_lib/scripts/convert.ts as well calls of these scripts in the project.json (l10n-create and l10n-convert)

apps/payments/next/Gruntfile.js
@@ -26,10 +26,6 @@ module.exports = function(grunt) {
src: srcPaths,
Copy link
Contributor

Choose a reason for hiding this comment

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

Yup it's still needed.

This grunt script creates the base payments-next.ftl file for base locale en which gets extracted by the l10n teams string extraction script, and passed on to Pontoon.

Additionally it also keeps the en/payments-next.ftl up to date during dev when new strings are added to the app.

@david1alvarez david1alvarez force-pushed the FXA-7841 branch 2 times, most recently from 1090ba2 to ab2d809 Compare January 14, 2025 20:53
@david1alvarez
feat(next): l10n: Update payments-next logic to use payments-next.ftl
31867a9 
Because:

* payments-next no longer needs to rely on payments translations, as payments-next translations are completed.

This commit:

* Removes the dependency on payments.ftl translations within payments-next project.json and gruntfile.js

Closes #FXA-7841
@david1alvarez david1alvarez merged commit a53e9a5 into main Jan 14, 2025
24 checks passed
@david1alvarez david1alvarez deleted the FXA-7841 branch January 14, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StaberindeZA StaberindeZA StaberindeZA approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@david1alvarez @StaberindeZA