-
Notifications
You must be signed in to change notification settings - Fork 919
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 overdue obsolete strings #14822
Conversation
8aa6b1d
to
ad339f1
Compare
ad339f1
to
97157a2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14822 +/- ##
==========================================
+ Coverage 77.29% 77.37% +0.08%
==========================================
Files 160 161 +1
Lines 8281 8302 +21
==========================================
+ Hits 6401 6424 +23
+ Misses 1880 1878 -2 ☔ View full report in Codecov by Sentry. |
8cbbb94
to
97157a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow what a cleanup job, thanks for taking a stab at this!
I'm adding a few more obsoletes that kinda never ended up being marked as obsolete: 😇
navigation-firefox-browsers-put = { -brand-name-firefox } browsers put your privacy first — and always have. | ||
navigation-take-the-passwords-youve = Take the passwords you’ve saved in { -brand-name-firefox } with you everywhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are gone too… FireTV, VR goggles, lockwise… isn't most of the v1 navigation gone already? (I see only bits like the navigation-firefox-blog at the end here being used as a fallback for navigation-v2-firefox-blog e.g., but otherwise this is probably all obsolete kinda implicitly, being removed at some point via nav redesign…)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I just noticed that too. I think I'll create a separate issue for this so this PR doesn't get too large!
Edit: created #14848!
{% extends "firefox/whatsnew/base.html" %} | ||
|
||
{% block page_title %}{{ ftl('whatsnew-page-title') }}{% endblock %} | ||
{% block page_title %}{{ ftl('whatsnew-page-title-v2') }}{% endblock %} | ||
|
||
{#- This will appear as <meta property="og:description"> which can be used for social share -#} | ||
{% block page_og_desc %}{{ ftl('whatsnew-page-description') }}{% endblock %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be easier to manage if all these titles + descriptions from all the wnp templates were moved to whatsnew/base?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just saw Craig's suggestion --
@craigcook is this still something you're hoping to work on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I forgot about that almost as soon as I posted it... but we've been talking about redoing the base WNP templates for some time and it just keeps getting put off for other work. I do think at the very least we can add the title string to the base file, that doesn't have to wait. If you want to do it as part of this PR that seems sensible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will be doing some light housekeeping in wnps, welcomes and subnavs soon~ish, so you can leave it like this and i'll pick it up later, possibly together with the v1 nav obsoletes…
(Just wanted to confirm it'd be a welcome change…)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a welcome change, thanks Jan!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R+ ✂️
One-line summary
Removing overdue obsolete strings from some Fluent files.
Note
I went ahead and removed an obsolete string expiring on July 22nd 2024, so we can have this PR merge on the 22nd
Issue
Fixes #14820
Testing