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 unused i18n keys #2086

Merged
merged 4 commits into from
Apr 27, 2021
Merged

Remove unused i18n keys #2086

merged 4 commits into from
Apr 27, 2021

Conversation

ChrisBAshton
Copy link
Contributor

This PR adds the i18n-coverage gem to monitor untested i18n keys. I've then removed used the coverage reports to identify and remove unused keys. See commits for details.

Trello: https://trello.com/c/Z3Pitrfg/2454-identify-and-remove-unused-keys-in-locale-files-5

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

This is the gem used by Content Publisher to track which i18n keys
are untested (and therefore possibly unused). The coverage report
is generated in `coverage/i18n`, which is git-ignored.
Removes unused `common.last_updated` field. I did consider using
this in published-dates.html.erb, but the string prefix with the
date varies from language to language, so this is better handled
in the (previously unused) `components.published_dates`'
`last_updated` and `published` fields.

At time of writing, these fields don't exist outside of en.yml,
which may be the reason behind the mixture of unused fields and
hardcoded text. We will standardise these keys later.
These keys, identified by the "i18n-coverage" gem, don't appear to
be used anywhere.

There are a number of other keys which the gem complains about,
but these are dynamically referenced so are difficult to statically
analyse. My assumption is that they are used.
@bevanloon bevanloon temporarily deployed to government-f-i18n-ptlrzvc1iatq April 23, 2021 16:00 Inactive
See 5e768df for commit which
removed these keys from the English locale. This commit removes
the same keys from all other locale files.
@bevanloon bevanloon temporarily deployed to government-f-i18n-ptlrzvc1iatq April 23, 2021 16:11 Inactive
@thomasleese
Copy link
Contributor

Do we know if there's any risk or problems with lines like

I18n.t("content_item.schema_name.#{schema_name}", count: 1, locale: :en)
causing us to think a translation isn't used when actually it is?

@ChrisBAshton
Copy link
Contributor Author

Do we know if there's any risk or problems with lines like

I18n.t("content_item.schema_name.#{schema_name}", count: 1, locale: :en)

causing us to think a translation isn't used when actually it is?

Yes, that is a risk. I manually searched for portions of the key, rather than the entire key, so that I could spot programmatic occurrences like this, in which case I've kept the keys rather than removed them.

I think the bigger risk is that there are some schema names in the i18n which aren't ever called upon programmatically. If, for example, there is no national schema, then the national key would be redundant, but there's no way to easily check. So the risk is we're not deleting enough (rather than we're deleting too much).

@ChrisBAshton ChrisBAshton merged commit f534c13 into master Apr 27, 2021
@ChrisBAshton ChrisBAshton deleted the i18n branch April 27, 2021 08:17
@benthorner
Copy link
Contributor

I spotted a slight issue with the config here while testing: #2112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants