The current build system used an incomplete integration of the KI18n
macro file.
Details
Details
compiled and did run make test to check autotests
Diff Detail
Diff Detail
- Repository
- R249 KI18n
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
This seems quite sensible to me, a cleanup and simplification of the test build system is not bad...
That said, i'm unsure why you think i'm qualified to speak about ki18n and the general cmake build system to a level high enough that you highlight me specifically to review this. As opposed to, you know, someone who actually worked on either of those two... ever ;)
Comment Actions
Sorry, copy and paste bug
It is complicate to find the correct reviewer for a project, because by default reviews do not get any reviewer by default. .arcconfig located in a related git repo clone does not contain any hint.
Comment Actions
Most of the projects are configured so that the reviews are automatic subscription to the relevant list. This means that the people involved with the project should already know about it, even without a reviewer and despite the message from phabricator UI.
Comment Actions
Any hint how to configure these subscriptiion - I saw a few projects I'm working on (umbrello, kmymony and alkimia) not having that configured, which is useful and https://community.kde.org/Infrastructure/Phabricator#Posting_a_Patch_using_the_website does not give any howto
Comment Actions
Setting up automatic notifications requires permission to alter the global Herald rules, which only a member of Community Admins can do.
Please ping one of them or file a Sysadmin ticket and we can sort that out.
Comment Actions
Looks good to me, I don't really see why it's better though...
Actually calling find_package on KF5I18n will define twice the targets such as KF5::I18n, I'm not sure this will even work alright.
Comment Actions
I don't understand either, what does this fix? Is cmake run separately on autotests/ki18n_install/CMakeLists.txt?
Comment Actions
This way you can find problems that are normally detected by client packages simply by running
make test
Actually calling find_package on KF5I18n will define twice the targets such as KF5::I18n, I'm not sure this will even work alright.
autotests/ki18n_install/CMakeLists.txt is a client package, which checks finding a ki18n package from the current build
Comment Actions
The macro file is intended to be used by client packages and is normally fetched by KI18nConfig.cmake, which is provided with this patch. Without this patch the test uses the Macro file directly in a different cmake context, which may hide issues.
Is cmake run separately on autotests/ki18n_install/CMakeLists.txt?
yes, by running
cd <ki18n-builddir> make test
or with more details
cd <ki18n-builddir> ctest -VV ki18n_install
Comment Actions
Ah, I see. This fails indeed:
CMake Error at CMakeLists.txt:4 (include):
include could not find load file:
KF5I18nMacros