Review Request 117823: Add ecm_install_po_files_as_qm() function

[Aurélien Gâteau]

> On None, Aurélien Gâteau wrote:
> > (commenting my own RR to split the discussion from the description)
> > 
> > I expect this function to become the most used one from this CMake module (together with ecm_create_qm_loader()) and the ecm_create_qm_from_po_files() to be less relevant. It is odd to name the CMake module after the less relevant function.
> > 
> > What do you think of doing the following?
> > 
> > - Create a new ECMPoQmTools.cmake file, and do the following in it:
> > -- Add ecm_install_po_files_as_qm()
> > -- Move ecm_create_qm_loader() into it
> > -- Create a ecm_process_po_files_as_qm() function, which would have the same signature as gettext_process_po_files() (can be done now because it makes sense to pass the language as first argument)
> > 
> > - Modify ECMCreateQmFromPoFiles.cmake like this:
> > -- Revert the latest changes to ecm_create_qm_from_po_files()
> > -- Include ECMPoQmTools.cmake (so that ecm_create_qm_loader() is still available when including the old file)
> > -- Mark the whole module as deprecated by adding a 'message(DEPRECATION "ECMCreateQmFromPoFiles.cmake is deprecated. Use ECMPoQmTools.cmake instead.")' at the start of the module.
> 
> Alex Merry wrote:
>     Well, you can't use DEPRECATION, as that doesn't exist in CMake 2.8.12. Otherwise, I think this is sensible. I'm definitely in favour of following the gettext_process_po_files() signature.
>     
>     I'm inclined to remove ECMCreateQmFromPoFiles.cmake entirely before 1.0, though (but maybe leave it in with an AUTHOR_WARNING for beta2, just to cause fewer issues for people updating only parts of their tree). It's not like anything other than some of the frameworks has made use of it.
>     
>     One thing I will say is: unit tests, please! See tests/ECMInstallIconsTest/ for inspiration.

I agree with removing ECMCreateQmFromPoFiles.cmake entirely before 1.0 and output a warning (using a flag which exists in cmake 2.8.12 :)).
Going to do the changes then, and write unit tests as well (thanks for the pointer).


- Aurélien


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


On April 28, 2014, 11:07 a.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117823/
> -----------------------------------------------------------
> 
> (Updated April 28, 2014, 11:07 a.m.)
> 
> 
> Review request for Build System, Extra Cmake Modules and KDE Frameworks.
> 
> 
> Repository: extra-cmake-modules
> 
> 
> Description
> -------
> 
> This patch adds a ecm_install_po_files_as_qm() function to ECMCreateQmFromPoFiles.cmake. This function makes it easy to install all the .po files of a Qt-translated framework.
> 
> The primary user of this function is the build system of the framework tarballs. The tarballs will ship with a po/ directory which follows the structure expected by ecm_install_po_files_as_qm(). The only necessary change is to add the following lines to the root CMakeLists.txt file:
> 
>     if (IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/po")
>         ecm_install_po_files_as_qm(po)
>     endif()
> 
> Since the code checks for the presence of a po/ directory, it can be committed in the framework repository (no need for the tarball release scripts to alter the CMake files), making it easier for developers to reproduce the situation of a released tarball by creating/copying a po/ directory in their working copy.
> 
> We need this function (or something similar) because the way we are going to organize po files in the framework tarballs have changed: the current CMake code expects the po files to contain files named '<domain>-<lang>.po', but the final structure is going to be '<lang>/<domain>.po'.
> 
> This new function works similarly to ki18n_install() ( https://git.reviewboard.kde.org/r/117804/ ) and kdoctools_install() ( https://git.reviewboard.kde.org/r/117805/ )
> 
> 
> Diffs
> -----
> 
>   modules/ECMCreateQmFromPoFiles.cmake 7457fc5 
> 
> Diff: https://git.reviewboard.kde.org/r/117823/diff/
> 
> 
> Testing
> -------
> 
> Modified kbookmarks to call ecm_install_po_files_as_qm(). Works as expected.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20140430/4843c527/attachment.html>