mozilla / bedrock

Making mozilla.org awesome, one pebble at a time
https://www.mozilla.org
Mozilla Public License 2.0
1.17k stars 912 forks source link

L10n automation doesn't seem to be removing obsolete files from "en" folder #13811

Open flodolo opened 10 months ago

flodolo commented 10 months ago

CC @peiying2

Comparing the en folder on www-l10n and bedrock, there are a set of extra files that were removed from Bedrock at some point, but were never removed from the localization repository (therefore, they're still exposed at least in Smartling).

404-locale.ftl
firefox/features/bookmarks.ftl
firefox/features/fast.ftl
firefox/features/independent.ftl
firefox/features/index.ftl
firefox/features/memory.ftl
firefox/features/password-manager.ftl
firefox/features/private-browsing.ftl
firefox/features/translate.ftl
firefox/retention/thank-you.ftl
firefox/welcome/page3.ftl
firefox/whatsnew/whatsnew-101-vpn-mobile.ftl
firefox/whatsnew/whatsnew-104-default.ftl
firefox/whatsnew/whatsnew-111-pdf.ftl
firefox/whatsnew/whatsnew-112-translate.ftl
firefox/whatsnew/whatsnew-98-vpn-eu.ftl
mozorg/home-mr2-promo.ftl
products/vpn/more/ip-address.ftl
products/vpn/more/vpn-or-proxy.ftl
products/vpn/more/what-is-a-vpn.ftl
products/vpn/more/when-to-use-a-vpn.ftl

Looks like the automation is adding new files but not removing obsolete ones?

flodolo commented 7 months ago

Any update on this issue? Just noticed a page removed, which is going to remain in the l10n repo unless manually removed.

stevejalim commented 6 months ago

Maybe the easiest thing here is to ensure developers also remove the l10n files when they drop a page?

Any thoughts on this @alexgibson ?

alexgibson commented 6 months ago

We add new files a lot more than we remove them, so if removing unneeded files manually is less work than it would take to set up automation for it, then that makes sense to me.

flodolo commented 6 months ago

The challenge, if I looked at the right code, is that you're syncing the entire l10n folder, which only has configs, en, en-US in Bedrock (and .toml files in the roo). So, using the purge setting is not an option. Maybe using the only with patterns is good enough? With that said, the whole regular expression thing in that command is confusing (why not glob patterns?).

stephaniehobson commented 5 months ago

I will make a re-occurring task to remove the files and add some documentation on how to.

So... how do we do this manually?

I assume a pull request to https://github.com/mozilla-l10n/www-l10n

I vaguely remember that the English files are used as the guide for what is and is not available for translation. Do we only need to delete the en ones? Or do the en-US ones need to go too?

flodolo commented 5 months ago

Removing from en is enough, I can create a workflow that periodically removes files from the l10n repository.

alexgibson commented 5 months ago

These files should have been removed from bedrock first if they are no longer needed. Because they were removed from the L10n repo instead, the PR automation is now trying to re-add the files again automatically https://github.com/mozilla-l10n/www-l10n/pull/384

Bedrock is the source of truth for en Fluent files. Whatever is in this repository, will be copied back over to the L10n repo via automation. Either we should revert https://github.com/mozilla-l10n/www-l10n/pull/382, or we should remove the files from bedrock too (the latter is probably easier now)

alexgibson commented 5 months ago

It might also be possible that some of the files removed in https://github.com/mozilla-l10n/www-l10n/pull/382 are actually still in use?

flodolo commented 5 months ago

It might also be possible that some of the files removed in mozilla-l10n/www-l10n#382 are actually still in use?

These are the files the PR is trying to re-create, which means they still exist in Bedrock's en folder. To be honest, it's been too long since I generated the list in the first comment, so not sure why the difference (script error, something else changed).

firefox/features/translate.ftl
products/vpn/more/ip-address.ftl
products/vpn/more/vpn-or-proxy.ftl
products/vpn/more/what-is-a-vpn.ftl
products/vpn/more/when-to-use-a-vpn.ftl

This is the diff I generated the other day

> diff -r www-l10n/en bedrock/l10n/en | grep ".ftl"

Only in www-l10n/en: 404-locale.ftl
Only in www-l10n/en/banners: fundraising.ftl
Only in www-l10n/en/banners: vpn-cyber-security-month.ftl
Only in www-l10n/en/banners: vpn-holidays.ftl
Only in www-l10n/en/firefox/browsers/compare: ie.ftl
Only in www-l10n/en/firefox/browsers/mobile: compare.ftl
Only in www-l10n/en/firefox/features: bookmarks.ftl
Only in www-l10n/en/firefox/features: fast.ftl
Only in www-l10n/en/firefox/features: independent.ftl
Only in www-l10n/en/firefox/features: index.ftl
Only in www-l10n/en/firefox/features: memory.ftl
Only in www-l10n/en/firefox/features: password-manager.ftl
Only in www-l10n/en/firefox/features: private-browsing.ftl
Only in www-l10n/en/firefox: firstrun.ftl
Only in www-l10n/en/firefox: sticky-promo.ftl
Only in www-l10n/en/firefox/welcome: page3.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-101-vpn-mobile.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-104-default.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-111-pdf.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-112-translate.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-115-vpn.ftl
Only in www-l10n/en/firefox/whatsnew: whatsnew-98-vpn-eu.ftl
Only in www-l10n/en/mozorg: home-mr2-promo.ftl
flodolo commented 5 months ago

Restored files in https://github.com/mozilla-l10n/www-l10n/pull/385

Luckily we didn't remove them from locales' folders, so it's annoying but not disruptive.

flodolo commented 5 months ago

Removed files that were only in the l10n repo and not available anymore in Bedrock: https://github.com/mozilla-l10n/www-l10n/pull/386

Alex fixed the config change: https://github.com/mozilla/bedrock/pull/14340#pullrequestreview-1948889512