[Freeipa-devel] [PATCH 70] validate i18n strings when running "make lint"

[John Dennis]

On 04/19/2012 07:04 AM, Petr Viktorin wrote:
> On 04/18/2012 09:32 PM, John Dennis wrote:
>>> Now that there are warnings, is pedantic mode necessary?
>>
>> Great question, I also pondered that as well. My conclusion was there
>> was value in separating aggressiveness of error checking from the
>> verbosity of the output. Also I didn't think we wanted warnings showing
>> in normal checking for things which are suspicious but not known to be
>> incorrect. So under the current scheme pedantic mode enables reporting
>> of suspicious constructs. You can still get a warning in the normal mode
>> for things which aren't fatal but are provably incorrect. An example of
>> this would be missing plural translations, it won't cause a run time
>> failure and we can be definite about their absence, however they should
>> be fixed, but it's not mandatory they be fixed, a warning in this case
>> seems appropriate.
>
> If they should be fixed, we should fix them, and treat them as errors
> the same way we treat lint's warnings as errors. If the pedantic mode is
> an obscure option of some test module, I worry that nobody will ever run it.

The value of pedantic mode is for the person maintaining the 
translations (at the moment that's me). It's not normally needed, but 
when something goes wrong it may be helpful to diagnose what might be 
amiss, in this case false positives are tolerable, in normal mode false 
positives should be silenced (a request of yours from an earlier 
review). Another thing to note is that a number of the warnings are 
limited to po checking, once again this is a translation maintainer 
feature, not a general test/developer feature.

> Separating aggressiveness of checking from verbosity is not a bad idea.
> But since we now have two severity levels, and the checking is cheap,
> I'm not convinced that the aggressiveness should be tunable.
> How about always counting the pedantic warnings, but not showing the
> details? Then, if such warnings are found, have the tool say how to run
> it to get a full report. That way people will notice it.

In an earlier review you asked to limit the output to only what is an 
actual provable error. I agreed with you and modified the code. One 
reason not to modify it again is the amount of time being devoted to 
polishing what is an internal developer tool. I've tweaked the reporting 
3-4 times already, I don't think it's time well spent to go back and do 
it again. After all, this is an internal tool, it will never be seen by 
a customer, if as we get experience with it we discover it's needs 
tweaking because it's not doing the job we hoped it would then that's 
the moment to invest more engineering resources on the output, 
validation, or whatever the deficiency is.

-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/