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

[Petr Viktorin]

On 04/19/2012 08:35 PM, John Dennis wrote:
> 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.

Thanks for the clarification. Now I see the use.

>> 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.
>


ACK (it's a 3.0 task, please push to master only)


-- 
Petr³