mozilla / pontoon

Mozilla's Localization Platform
https://pontoon.mozilla.org
BSD 3-Clause "New" or "Revised" License
1.47k stars 528 forks source link

Option to add exceptions for strings in WARNINGS category #2143

Open bugzilla-to-github opened 6 years ago

bugzilla-to-github commented 6 years ago

This issue was created automatically by a script.

Bug 1493876

Bug Reporter: @a-polivanchuk CC: @flodolo, @Pike, @mathjazz

The new category WARNINGS should be improved. For the UK locale, we have warnings which are reviewed and approved. They are mostly related to the strings where the placeholder is replaced by text because of the known nominative case issue.

Translators are starting to submit suggestions for the strings in WARNINGS category. They see missing placeholder and think that it should be fixed.

The option to add exceptions for such strings would solve this. Also, we have to keep in mind that such improvement can cause another issue: How to handle exceptions?

bugzilla-to-github commented 6 years ago

Comment Author: @flodolo

I agree that we should allow locales to mark warnings as false positive, but I believe that requires a lot more than Pontoon.

It should be possible to ignore some warnings by adding a semantic comment in the source file. Pontoon and dashboards should be able to read that information, and write it if needed. Just fixing it in Pontoon wouldn't be enough.

P.S. As for the 7 warnings showing up in Firefox for uk, I'm positive I can help you get rid of them. Please drop me an email explaining cases, how that changes the spelling of each Sync/Firefox Account, and I'll guide you through it.

bugzilla-to-github commented 6 years ago

Comment Author: @mathjazz

Stupid question: why not do it in Pontoon only? For example with an "Ignore warnings" option, which would make Translations with warnings act like they were Approved. It would have no impact on VCS, compare-locales, dashboards, product...

If the concern is that Pontoon dashboard and Elmo dashboard numbers will no longer match, we should be able to solve this through the API.

bugzilla-to-github commented 6 years ago

Comment Author: @flodolo

In a sense, it would make Pontoon the source of truth, not VCS. And we need to all agree if that's OK.

If we decide we can have this only in Pontoon, then we still need a way which checks were disabled.

bugzilla-to-github commented 6 years ago

Comment Author: @flodolo

(In reply to Francesco Lodolo [:flod] from comment #3)

If we decide we can have this only in Pontoon, then we still need a way which checks were disabled.

a way to know which checks were disabled (e.g. filters)

bugzilla-to-github commented 6 years ago

Comment Author: @mathjazz

(In reply to Francesco Lodolo [:flod] from comment #3)

In a sense, it would make Pontoon the source of truth, not VCS. And we need to all agree if that's OK.

Warnings are not stored in VCS, so I don't think that's the case. And VCS is the source of truth only for data stored in VCS.

Independently of that: the problem this bug tries to address is Pontoon specific. ATM I count 46 Warnings with unreviewed suggestions submitted since we landed checks on dashboards. Most if not all of them are probably redundant. I'm not aware of false positive Warnings causing a problem of similar scale and urgency outside Pontoon.

Further down the road I agree it makes sense to consider using something like semantic comments to designate ignored warnings, but there's a pile of nuts to crack to get there. One of them is adding semantic comments support to Fluent (and then to the rest of the toolchain), the other is extending Fluent support across all projects we run checks on. That will take time.

I wonder what :Pike thinks about all that (CCing).

If we decide we can have this only in Pontoon, then we still need a way to know which checks were disabled (e.g. filters).

Agreed.

bugzilla-to-github commented 6 years ago

Comment Author: @flodolo

(In reply to Matjaz Horvat [:mathjazz] from comment #5)

(In reply to Francesco Lodolo [:flod] from comment #3)

In a sense, it would make Pontoon the source of truth, not VCS. And we need to all agree if that's OK.

Warnings are not stored in VCS, so I don't think that's the case. And VCS is the source of truth only for data stored in VCS.

The moment we introduce the possibility to skip warnings, we'll be storing relevant information only in Pontoon.

We have already been talking about allowing that in general, through semantic comments, since warnings will keep showing up on the l10n dashboard, and there's no way to avoid that. Sure, Pontoon could be fixed as a first step in that direction.

For the record, I believe we're going to have the same conundrum the moment we introduce a feedback cycle, where all that information – or most of it – will be stored outside of VCS.

Independently of that: the problem this bug tries to address is Pontoon specific. ATM I count 46 Warnings with unreviewed suggestions submitted since we landed checks on dashboards. Most if not all of them are probably redundant. I'm not aware of false positive Warnings causing a problem of similar scale and urgency outside Pontoon.

I don't think they're all false positives, only a handful of locales introduce those on purpose. So I believe we're looking at the problem from the wrong perspective.

Most of the warnings come from migrations, where it's impossible to replace a declined brand name like Firefox Account and Sync, and those can be fixed properly with Fluent. https://l10n.mozilla.org/source/diff/?from=4c9189773f54&to=1f1775bb13ab&repo=l10n-central%2Fuk

Others come from translating brand names like Firefox and Mozilla, and we sadly should fix those as well.

On a side note, as a reviewer, I feel like just allowing to mark a warning as false positive is going to take a step back in terms of quality. Localizers will mark a warning as irrelevant, then I'll revert that, and then back to step 1 if they ignore it again.

As said, I believe that most warnings should be fixed. For others, the first step would be to allow adding comments to the string ("This brandname is omitted on purpose because X"). And that's part of the larger discussion about feedback cycle.