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

Differentiate empty from missing strings #768

Merged
merged 5 commits into from
Jul 12, 2016
Merged

Conversation

flodolo
Copy link
Collaborator

@flodolo flodolo commented Jul 6, 2016

Fix #767

@@ -34,7 +34,7 @@
" <th><a href='#{$locale}'>{$locale}</a></th>\n";

if (! $translation) {
echo " <td><em class='error'>Warning: Missing string</em></td><td></td>\n";
echo " <td><em class='error'>Warning: Empty string</em></td><td></td>\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m confused why you left only “Empty”. Now on http://localhost:8082/string/?entity=dom/chrome/dom/dom.properties:GenericImageNamePNG&repo=central it’s displaying "Empty string” everywhere while the string is missing.

@TheoChevalier
Copy link
Member

Not sure if there’s something missing, but I tried a few queries with a brand new dev server (data got corrupt on my full install), and I see “Warning: Empty string” everywhere. Even for Venkman strings for instance, and they are gone from locale repos

@flodolo
Copy link
Collaborator Author

flodolo commented Jul 8, 2016

There something wrong with my local VM, I switched the dev server to my branch and I see your same errors.

Will need to look into it next week.

@flodolo
Copy link
Collaborator Author

flodolo commented Jul 8, 2016

You're right about showing "Missing string" in onestring, I can't understand why the online servers are still showing Venkman, when they shouldn't
#709

@flodolo
Copy link
Collaborator Author

flodolo commented Jul 8, 2016

Let's close this one. I realized it needs a lot more work, starting from extraction :-\

@flodolo flodolo closed this Jul 8, 2016
@flodolo flodolo reopened this Jul 8, 2016
@flodolo
Copy link
Collaborator Author

flodolo commented Jul 8, 2016

This might be ready for some more tests. To test, you need to recreate TMX caches, you can do that setting forceTMX to true in glossaire.sh
https://github.com/mozfr/transvision/blob/master/app/scripts/glossaire.sh#L111

One string (view and API): it should only show the strings we actually have, or strings that are empty.

I tried to go through the code and identify places where we assumed empty=missing, but I only found places where we wrongly assumed that missing strings would be missing from the TMX.

Changes to tmx_products.py are also in its own repo upstream, with tests
flodolo/tmx_maker#4

@TheoChevalier
Copy link
Member

Looks good 👍 We’ll have to remember recreating TMX files on the server for the next release

@TheoChevalier TheoChevalier merged commit 64dda7a into mozfr:master Jul 12, 2016
@flodolo flodolo deleted the empty branch July 12, 2016 10:13
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.

2 participants