Needed for D28608
Details
Diff Detail
- Repository
- R85 PIM: Kontact Interface
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 24878 Build 24896: arc lint + arc unit
KF5KontactInterfaceConfig.cmake.in | ||
---|---|---|
6 | You might also consider using https://api.kde.org/ecm/module/ECMSetupQtPluginMacroNames.html now over manual code, given you depend on new enough ECM. |
Use ECMSetupQtPluginMacroNames.
Not sure I see the point in writing 6+1 lines in order to generate 3
(or 3+1 vs 3 with less wrapping), but OK :)
Yes, it makes more sense in projects which use the macro also in the same project. Here I proposed it more for the sake of consistency, and promoting its existance, so others just copying code would learn about it.
CMakeLists.txt | ||
---|---|---|
43 | You wanted "EXPORT_KONTACT_PLUGIN_WITH_JSON" here :) |
Actually, (been some time, only remembered now) the macro ensures that one does not overlook the usefulness to also have CMAKE_AUTOMOC_DEPEND_FILTERS set to help automoc to track the depenedency to the JSON file when used, like with the new macro, as moc should be rerun when the JSON file changed.... ;)
Ah, changes to the json file not being taken into account, that's indeed a real problem with the old way. You're saying the new way fixes that?
Seems so indeed. Great!
"Looks" okay to me from pure code reading for what ecm_setup_qtplugin_macro_names matters. Cannot test though, no PIM devel setup sadly.
CMakeLists.txt | ||
---|---|---|
38 | I would group the include(ECMSetupQtPluginMacroNames) with the other includes for consistency. | |
src/plugin.h | ||
67 | Perfect would be to document the paramters also :) | |
69–78 | What is the purpose of pluginname? |
src/plugin.h | ||
---|---|---|
69–78 | Wow, unused. Must be a leftover.... I'll remove it. Well spotted. |