> On March 27, 2014, 8:31 p.m., Alex Merry wrote: > > modules/ECMSetupQtTranslations.cmake, lines 81-83 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257836#file257836line81> > > > > FindGettext calls the target pofiles. Is it worth making this target > > qmfiles for consistency? > > Aurélien Gâteau wrote: > "pofiles" is a wrong name: FindGettext code does not produce .po files it > produce .mo files. So it's not that consistent already. I don't mind renaming > it though if you prefer.
Oh, actually, I was mixing up the Gettext macros. The equivalent Gettext macro creates a "translations" target, which seems sensible to me. > On March 27, 2014, 8:31 p.m., Alex Merry wrote: > > modules/ECMSetupQtTranslations.cmake, line 97 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257836#file257836line97> > > > > Please use the style > > set(ecm_sqt_options) > > set(ecm_sqt_oneValueArgs ARGS HERE) > > set(ecm_sqt_multiValueArgs) > > cmake_parse_arguments(ECM_SQT "${ecm_sqt_options}" > > "${ecm_sqt_oneValueArgs}" "${ecm_sqt_multiValueArgs}" ${ARGN}) > > as this makes it much clearer what's going on. > > > > Also, make sure global variables start with ECM_ (so ECM_SQT here), so > > we can guarantee not to clash with any other packages. > > Aurélien Gâteau wrote: > There is no need to prefix variables if ecm_setup_qt_translations is > turned into a function though, right? No, you can use whatever names you like. > On March 27, 2014, 8:31 p.m., Alex Merry wrote: > > modules/ECMTrLoader.cpp.in, line 19 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257837#file257837line19> > > > > QLatin1String() when you're using + > > Aleix Pol Gonzalez wrote: > Why? That doesn't make sense on my book. > > Alex Merry wrote: > *Goes and reads the docs again* > > Ah, I think you're right there. QLatin1String is good for things that > never get converted, like ==, but otherwise the Literal works better. > > You can just drop this issue. Although in this case, QLatin1Char might be even better. Not that the efficiency gain will matter that much. > On March 27, 2014, 8:31 p.m., Alex Merry wrote: > > modules/ECMSetupQtTranslations.cmake, line 7 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257836#file257836line7> > > > > I think the name should somehow indicate that it's for integrating a > > gettext-based workflow into Qt's l10n support. A macro to facilitate a > > Linguist-based workflow would also be reasonable (not that KDE would have > > much use for it). > > Aurélien Gâteau wrote: > I agree, but can't figure out a proper name. Any ideas? ecm_create_qt_translations_from_po_files? That is it's core functionality (and the loader stuff is an optional extra). - Alex ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117052/#review54369 ----------------------------------------------------------- On March 28, 2014, 9:45 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 28, 2014, 9:45 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/ECMSetupQtTranslations.cmake PRE-CREATION > modules/ECMTrLoader.cpp.in 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
