Remove unused locale messages #7190
Conversation
Gudahtt
force-pushed
the
cleanup-locales
branch
2 times, most recently
from
5b3492b to
05f56e0
Compare
|
Here's the list of removed messages:
Edit: this as been updated to remove messages that were missed because they used a string template, explained here. Edit: |
|
I've found these additional messages that have corresponding string literals, but seem to be unused:
|
Various message keys were being specified with a string template instead of a string literal. They have been switched to use string literals so that the script for detecting unused messages can find them.
Gudahtt
force-pushed
the
cleanup-locales
branch
from
05f56e0 to
034a750
Compare
|
I've just updated this to restore a few messages on the
|
Gudahtt
force-pushed
the
cleanup-locales
branch
from
034a750 to
ae3a84a
Compare
A number of unused locale messages have been removed - probably leftover from old UI elements that have since been removed. The `verify_locale_strings` script has been augmented to search the UI for string literals, and match those against the locale message keys in the `en` locale. Any messages without a corresponding string literal are assumed to be unused. The script has also been updated with an optional `--fix` parameter, which will automatically delete any unused messages from locales. 148 unused messages were found in this case, out of a total of about 650 messages. Another 70 messages are _potentially_ unused and require further investigation, but weren't as easy to rule out because they were found in string literals.
The following messages were more difficult to rule out because they were present as string literals in the UI. They do appear to be unused as locale keys though. Here are the removed messages: - [ ] add - [ ] address - [ ] addressBook - [ ] editingTransaction - [ ] gas - [ ] gasFee - [ ] info - [ ] onlySendToEtherAddress - [ ] onlySendTokensToAccountAddress - [ ] restoreVault - [ ] rpc - [ ] status - [ ] tokenBalance - [ ] transactions - [ ] warning
Gudahtt
force-pushed
the
cleanup-locales
branch
from
ae3a84a to
2102594
Compare
|
This should be in a decent state to review now. I'm reasonably confident that everything in the first batch of messages I removed is truly unused. The second batch I'm less certain about - I certainly can't find them used anywhere, and I haven't noticed any console warnings about them being missing when using the app, but they might be worth some additional scrutiny. They're listed here: #7190 (comment) |
Builds ready [2102594]
|
| // | ||
| // will check the given locale against the strings in english | ||
| // This script will validate that locales have no unused messages. It will check |
There was a problem hiding this comment.
It sounds like we might want the inverse also, based on the concerns you've raised: Validate that no UI template call uses a non-existent message. Not sure how efficiently this can be done, but if we assume the template string is always in a t() call, it might be efficient enough to regex.
Just nervous about removing used messages, like you suggested might be the case!
There was a problem hiding this comment.
I tried this already but too many cases use variables instead of string literals.
Many of those cases could be eliminated pretty easy, but there are a few that are very dynamic that would be challenging to refactor.
A better way to catch these cases might be to send an error to Sentry when we detect a missing key from the en locale, rather than just print a console warning like we do now.
There was a problem hiding this comment.
Yeah, that'd be a good use of sentry.
* origin/develop: (56 commits) Add advanced setting to enable editing nonce on confirmation screens (MetaMask#7089) Add migration on 3box imports and remove feature flag (MetaMask#7209) ci - install deps - limit install scripts to whitelist (MetaMask#7208) Add a/b test for full screen transaction confirmations (MetaMask#7162) Update minimum Firefox verison to 56.0 (MetaMask#7213) mesh-testing - submit infura rpc requests to mesh-testing container (MetaMask#7031) obs-store/local-store should upgrade webextension error to real error (MetaMask#7207) sesify-viz - bump dep for visualization enhancement (MetaMask#7175) address book entries by chainId (MetaMask#7205) Optimize images only during production build (MetaMask#7194) Use common test build during CI (MetaMask#7196) Report missing `en` locale messages to Sentry (MetaMask#7197) Verify locales on CI (MetaMask#7199) updated ganache and addons-linter (MetaMask#7204) fixup! add user rejected errors add user rejected errors update json-rpc-engine use eth-json-rpc-errors Remove unused locale messages (MetaMask#7190) Remove unused components (MetaMask#7191) ...
A number of unused locale messages have been removed - probably leftover from old UI elements that have since been removed.
The
verify_locale_stringsscript has been augmented to search the UI for string literals, and match those against the locale message keys in theenlocale. Any messages without a corresponding string literal are assumed to be unused.The script has also been updated with an optional
--fixparameter, which will automatically delete any unused messages from locales.160 unused messages were found in this case, out of a total of about 650 messages. Another 70 messages are potentially unused and require further investigation, but weren't as easy to rule out because they were found in string literals.