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

Add W304 for untranslated messages #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Oct 13, 2021

No description provided.

@willkg
Copy link
Member

willkg commented Oct 18, 2021

This change undoes part of this fix:

8a1d4a8

I haven't worked on a project that needed translations in 5+ years, so I'm pretty far removed from this space. I don't remember why we had so many fuzzy strings, but I do remember getting yelled at for trying to clean them up.

@jayvdb
Copy link
Contributor Author

jayvdb commented Oct 18, 2021

My guess is that the .po files contained lots of obsolete entries, as those can accumulate over time, and likely include strings which have become obsolete because of improvements due to linter results. I agree those shouldn't be linted by default. If that sounds like the problem, I can achieve both by either:

-for entry in po:
+for entry in [e for e in po if not e.obsolete]:

or

 for entry in po:
+    if entry.obsolete:
+        continue

For context, I am trying to implement untranslated detection so it can be used in CD to ensure that the .po files for certain languages have been updated to 100% translated before a release is deployed, and have warnings in CI to show which translations are still missing so they can easily be sent off to the translators (via Excel .. :/). dennis seems like a good fit because of its ability to also check for translation errors, and the current tool we're using isnt compatible with Django (ziima/polint#8).

@willkg
Copy link
Member

willkg commented Oct 18, 2021

I need to remember how the PO library Dennis uses works. I'll look at that and get back to this some time this week.

@willkg
Copy link
Member

willkg commented Oct 19, 2021

It's curious that polint doesn't document .fuzzy, .obsolete, and other flags on POEntry.

I think you're right on and we want something like:

for entry in po:
    if entry.fuzzy or entry.obsolete:
        continue

I think that does what .translated_strings() is doing now, except it'll let through strings that aren't translated which lets your rule work.

Does that sound ok?

@willkg
Copy link
Member

willkg commented May 27, 2022

I didn't know you updated the commit. I'll look at it later today.

@willkg
Copy link
Member

willkg commented May 27, 2022

This needs some tests so we know it's working as you expect and we know when some future change breaks it. We also need to document the new warning.

I think I can do both of those things, but I'll need you to review the work because there's no description in this PR and I'll be guessing at things based on the comments. Is that ok? Or would you rather do the tests and docs?

@jayvdb
Copy link
Contributor Author

jayvdb commented May 28, 2022

I am happy to add tests and docs.

@willkg
Copy link
Member

willkg commented Jun 8, 2022

FYI, I renamed the default branch to "main" and I transferred this project to the Mozilla org.

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.

None yet

2 participants