Review Request 117052: Add ECMSetupQtTranslations

Aurélien Gâteau agateau at kde.org
Wed Mar 26 08:36:25 UTC 2014



> On March 25, 2014, 3:33 p.m., Alex Merry wrote:
> > The docs need cleaning up, but I'd like to concentrate on the API first.
> > 
> > I'd rather this followed the convention of other file-generating macros in getting the user to provide a variable name to store the file(s) in, rather than always using ECM_QT_TRANSLATION_LOADER.
> > 
> > I also think that the installation should be handled by the caller.  Provide a variable to store the qmfiles in, and require the caller to install them in a subdir <podir> of something in QStandardPaths::GenericDataLocation.  That way, the project can make use of the user-configurable DATA_INSTALL_DIR from KDEInstallDirs, for example, or their own installation path code.
> > 
> > I could imagine that an option to specify the name of the loader file could potentially be useful to avoid clashes, but let's leave that until someone says they want it.

I made the macro install files itself to avoid developers getting it wrong: it is very easy to forget to install translation files or to install them in the wrong place. Most developers won't notice it because they don't test translations.

Same with the variable name: the function could be modified to accept the name of the variable as argument, but that adds complexity and more chances of getting it wrong. I assume we don't want to support the possibility to call ecm_setup_qt_translations() twice within the same framework. If you think this is a valid use-case, though, then the macro needs to be made more customizable.

Nevertheless, you are the maintainer, so I will make any changes you feel are needed. Can you give me the prototype of the macro you would like to see and how one should use it?


- Aurélien


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117052/#review54111
-----------------------------------------------------------


On March 25, 2014, 11:48 a.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117052/
> -----------------------------------------------------------
> 
> (Updated March 25, 2014, 11:48 a.m.)
> 
> 
> Review request for Build System, Extra Cmake Modules and KDE Frameworks.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> This simplifies translation handling for frameworks using Qt translation system.
> 
> 
> Diffs
> -----
> 
>   modules/ECMTrLoader.cpp.in PRE-CREATION 
>   modules/ECMSetupQtTranslations.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/117052/diff/
> 
> 
> Testing
> -------
> 
> Here is a patch which make kbookmarks use it: http://agateau.com/tmp/kbookmarks-tr.diff . The necessary changes for the Messages.sh part of the patch are being reviewed for inclusion in the l10n-kde4 repository.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20140326/860d602b/attachment.html>


More information about the Kde-buildsystem mailing list