Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated June 22, 2015, 1:13 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Changes --- Submitted with commit 2fff683110385047bd20fb5f0b68a008b48320dd by Chusslove Illich (?? ) to branch master. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - autotests/kconfig_compiler/CMakeLists.txt f73aec0 autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 autotests/kconfig_compiler/klocalizedstring.h e69de29 autotests/kconfig_compiler/test_translation.kcfg e69de29 autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review81646 --- Ship it! LGTM, thanks for adding the unit tests! I have just one minor nitpick below (no need to repost the patch). Also please be sure to include a Changelog: line in the commit message. autotests/kconfig_compiler/test_translation.kcfg (line 4) https://git.reviewboard.kde.org/r/123872/#comment55962 Nitpick: trailing space - Matthew Dawson On June 21, 2015, 4:47 p.m., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated June 21, 2015, 4:47 p.m.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - autotests/kconfig_compiler/CMakeLists.txt f73aec0 autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 autotests/kconfig_compiler/klocalizedstring.h e69de29 autotests/kconfig_compiler/test_translation.kcfg e69de29 autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
On Јун 6, 2015, 10:21 пре п., David Faure wrote: A test doesn't really need ki18n, it could just define its own i18n() function. Alternatively, you can put the test in a higher level framework, like frameworkintegration. I took the first approach, by adding a fake klocalizedstring.h in the test directory. - Chusslove --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review81259 --- On Јун 21, 2015, 10:47 по п., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated Јун 21, 2015, 10:47 по п.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - autotests/kconfig_compiler/CMakeLists.txt f73aec0 autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 autotests/kconfig_compiler/klocalizedstring.h e69de29 autotests/kconfig_compiler/test_translation.kcfg e69de29 autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated Јун 21, 2015, 10:47 по п.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Changes --- Added unit tests. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs (updated) - autotests/kconfig_compiler/CMakeLists.txt f73aec0 autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 autotests/kconfig_compiler/klocalizedstring.h e69de29 autotests/kconfig_compiler/test_translation.kcfg e69de29 autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review81259 --- A test doesn't really need ki18n, it could just define its own i18n() function. Alternatively, you can put the test in a higher level framework, like frameworkintegration. - David Faure On May 21, 2015, 3:21 p.m., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated May 21, 2015, 3:21 p.m.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
On Мај 21, 2015, 5:29 по п., Matthew Dawson wrote: LGTM in general, but could you please add/update a unit test to test all the possible cominbations of the translation? That would make the test depend on the Ki18n framework, which is not acceptable for tier 1 frameworks. Alternatively, one could add fake klocalizedstring.h header, to shadow any system header, and fill it with fake translation calls. But such test would only check if generated translation call names are as expected, which is trivially obvious from the code. - Chusslove --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review80698 --- On Мај 21, 2015, 5:21 по п., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated Мај 21, 2015, 5:21 по п.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
On May 21, 2015, 11:29 a.m., Matthew Dawson wrote: LGTM in general, but could you please add/update a unit test to test all the possible cominbations of the translation? Chusslove Illich wrote: That would make the test depend on the Ki18n framework, which is not acceptable for tier 1 frameworks. Alternatively, one could add fake klocalizedstring.h header, to shadow any system header, and fill it with fake translation calls. But such test would only check if generated translation call names are as expected, which is trivially obvious from the code. Even if it doesn't actually compile the code, I'd like to have something verify the calls are actually being put in the generated file. It is trivial to see this RR is correct, but the problem is the future when someone (read: me) messes up the translation support. I'd like to have something verify that block does what it should in the future, especially if that block gets a large refactor. - Matthew --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review80698 --- On May 21, 2015, 11:21 a.m., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated May 21, 2015, 11:21 a.m.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/#review80698 --- LGTM in general, but could you please add/update a unit test to test all the possible cominbations of the translation? - Matthew Dawson On May 21, 2015, 11:21 a.m., Chusslove Illich wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123872/ --- (Updated May 21, 2015, 11:21 a.m.) Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson. Repository: kconfig Description --- When using Ki18n as the translation system, library .kcfg files also need to specify the translation domain. This is analogous to the TRANSLATION_DOMAIN define in C++ code, and translationDomain attribute in .rc files. Diffs - src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 Diff: https://git.reviewboard.kde.org/r/123872/diff/ Testing --- Compiles. Thanks, Chusslove Illich ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
