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

Remove old obsolete strings from Fluent files #13886

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

reemhamz
Copy link
Contributor

@reemhamz reemhamz commented Nov 13, 2023

One-line summary

Removing old (3+ months) obsolete Fluent strings from Fluent files and any mention of them from HTML files (usually placed in fallback helpers)

✍🏼 Things to note: ✍🏼

  • I left the obsolete strings related to the Mozilla accounts update + Monitor updates, since the new strings are still fresh, and we're still using the older strings as fallbacks. We can remove the obsolete strings start of Q1 2024
  • I updated WNP 114-119 to include the new title string that was created in a Fluent file but not used. I confirmed with Craig to update the files.

Issue / Bugzilla link

#13714

Testing

The following are pages whose HTML files were edited to remove old strings in fallback/ftl_has_messages helpers
Did I miss anything?

WNP page title string updates:

@reemhamz reemhamz marked this pull request as draft November 13, 2023 10:59
@reemhamz reemhamz added L10N WIP 🚧 Pull request is still work in progress labels Nov 13, 2023
@reemhamz reemhamz force-pushed the remove-obsolete-strings branch 3 times, most recently from 6baa05a to f288b25 Compare November 14, 2023 03:32
@reemhamz reemhamz added Review: M Code review time: 1-2 hours Needs Review Awaiting code review labels Nov 14, 2023
@reemhamz reemhamz marked this pull request as ready for review November 14, 2023 04:06
@reemhamz reemhamz removed the WIP 🚧 Pull request is still work in progress label Nov 16, 2023
@stephaniehobson stephaniehobson self-assigned this Nov 17, 2023
Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

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

Looks great. Very through 💯 Just needs a rebase to fix the conflicts.

@@ -248,7 +248,6 @@ <h3>{{ ftl('privacy-book-stay-safe-online') }}</h3>
<li>{{ ftl('privacy-book-some-online-services') }}</li>
<li>
<p>{{ ftl('privacy-book-wondering-how-youre') }}</p>
<p>{{ ftl('privacy-book-there-are-several-v2', fallback='privacy-book-there-are-several') }}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch. 🔒

@@ -6,7 +6,7 @@

{% extends "firefox/whatsnew/base.html" %}

{% block page_title %}{{ ftl('whatsnew-page-title') }}{% endblock %}
{% block page_title %}{{ ftl('whatsnew-page-title-v2' ,fallback='whatsnew-page-title') }}{% endblock %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The space is on the wrong side of the comma (for all the WNP titles)

If fixing this fills you with dread we can totally leave it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If fixing this fills you with dread we can totally leave it.

Well when you put it like that... 😝 j/k, I'll fix it

@reemhamz
Copy link
Contributor Author

reemhamz commented Nov 20, 2023

^ Seems like I also committed a file that was updated in this PR: #13900 by the rebase I just did 🤦🏻‍♀️, but it seems very minimal and harmless (since it's a small string update) so I'll just proceed and merge this PR

Copy link
Contributor

@stephaniehobson stephaniehobson left a comment

Choose a reason for hiding this comment

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

R+

@stephaniehobson stephaniehobson merged commit 1ad43b0 into main Nov 20, 2023
6 checks passed
@stephaniehobson stephaniehobson deleted the remove-obsolete-strings branch November 20, 2023 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L10N Needs Review Awaiting code review Review: M Code review time: 1-2 hours
3 participants