KDiagram libs (KChart, KGantt) in KDE Review
Friedrich W. H. Kossebau
kossebau at kde.org
Mon Feb 9 11:05:56 GMT 2015
Am Montag, 9. Februar 2015, 04:09:59 schrieb Aleix Pol:
> I just went through the cmake code.
> Let's see:
> - I see this, what does need to be fixed? > # TODO: fix
> ecm_generate_headers to support camelcase .h files
See https://git.reviewboard.kde.org/r/122317/ , and for related discussion
By current feddback in the other RR 122317 seems okay principally, just noone
has yet ship-it-ed. Looking forward to someone doing so, so KDiagram can soon
make use of the macro also for KChart (yes, could have also worked-around by
lowercasing all filenames in the sources, but did not want to change even more
things than just the namespace).
> - Library targets are usually called KF5*, see KF5Parts or KF5Gantt
Even if not part of KF5 itself? So should any libraries in KDE repos prefix
their targets like that, because the related ECM macros are used?
I would rather think KF5 prefix should be reserved to libs from KF5, but if
what you propose is already standard, I will follow.
> - Is this really needed? add_definitions(-DKDAB_NO_UNIT_TESTS). It's
> not very good to compile differently things if there's unit tests and
Not perfect, agreed. Okay if I add a TODO for now?
(given this is also in existing released code, and fixing might need some
> - some of the definitions in the root CMakeLists.txt files are already
> being defined by KDEFrameworkCompilerSettings. You might want to clean
> them up.
> - misses a .reviewboardrc file.
> I created a small review request you also want to look into:
Thanks for that, going to comment on tonight.
More information about the kde-core-devel