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

[Martin Kosek]

On Fri, 2012-04-20 at 10:14 +0200, Petr Viktorin wrote:
> 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)
> 
> 

Rebased and pushed to master.

Martin