On 04/09/2012 05:24 PM, John Dennis wrote:
On 03/30/2012 08:57 AM, Petr Viktorin wrote:
On 03/30/2012 02:41 AM, John Dennis wrote:
On 03/28/2012 04:40 AM, Petr Viktorin wrote:
Can install/po/Makefile just call test_i18n.py from the tests/ tree? It
doesn't import any IPA code so there's no need to set sys.path in this
case (though there'd have to be a comment saying we depend on this).
In the other case, unit tests, the path is already set by Nose.
Also the file would have to be renamed so nose doesn't pick it up as a
test module.

Good idea. I moved test_i18n.py to tests/i18n.py. I was reluctant about
moving the file, but that was without merit, it works better this way.

The downside is that the file now looks like a test utility module. It
could use a comment near the imports saying that it's also called as a
script, and that it shouldn't import IPA code (this could silently use
the system-installed version of IPA, or crash if it's not there).
Alternatively, set PYTHONPATH in the Makefile.

I also removed the superfluous comment in Makefile.in you pointed out.

When I was exercising the code I noticed the validation code was not
treating msgid's from C code correctly (we do have some C code in the
client area). That required a much more nuanced parsing the format
conversion specifiers to correctly identify what was a positional format
specifier vs. an indexed format specifier. The new version of the
i18n.py includes the function parse_printf_fmt() and get_prog_langs() to
identify the source programming language.

More non-trivial code without tests. This makes me worry. But, tests for
this can be added later I guess.

Two more patches will follow shortly, one which adds validation when
"make lint" is run and a patch to correct the problems it found in the C
code strings which did not used indexed format specifiers.

revised patch with comment about imports attached.



ACK

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to