> On March 27, 2014, 9: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?
"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. > On March 27, 2014, 9:31 p.m., Alex Merry wrote: > > modules/ECMSetupQtTranslations.cmake, line 96 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257836#file257836line96> > > > > Why is this a macro, rather than a function? Don't forget the > > PARENT_SCOPE argument of set() Oh, I didn't know about PARENT_SCOPE. Nice. > On March 27, 2014, 9: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. There is no need to prefix variables if ecm_setup_qt_translations is turned into a function though, right? > On March 27, 2014, 9:31 p.m., Alex Merry wrote: > > modules/ECMSetupQtTranslations.cmake, line 43 > > <https://git.reviewboard.kde.org/r/117052/diff/2/?file=257836#file257836line43> > > > > Is it worth filing a Qt bug? If there already is one, please put the > > link in the comment. > On March 27, 2014, 9: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). I agree, but can't figure out a proper name. Any ideas? - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117052/#review54369 ----------------------------------------------------------- On March 27, 2014, 4:26 p.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 27, 2014, 4:26 p.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