KDiagram libs (KChart, KGantt) in KDE Review
aleixpol at kde.org
Mon Feb 9 03:09:59 GMT 2015
On Mon, Feb 9, 2015 at 1:50 AM, Friedrich W. H. Kossebau
<kossebau at kde.org> wrote:
> Am Montag, 9. Februar 2015, 00:23:58 schrieb Albert Astals Cid:
>> El Diumenge, 8 de febrer de 2015, a les 00:29:26, Friedrich W. H. Kossebau
>> > Hi,
>> > Calligra, Massif-Visualizer, KMyMoney (and perhaps more) make use of
>> > KDAB's
>> > nice offer of their KDChart in the GPLv2 licensing variant. But as there
>> > are no real public releases of KDChart, all the projects have copied a
>> > dump of KDChart into their code repositories, updating now and then to
>> > newer versions of KDChart. Which is anything but perfect.
>> > To improve things, after some talk with KDABians it was decided to create
>> > a
>> > KDE repo with a KDE-fied fork of KDChart, based on their latest Qt5ied
>> > version. So all FLOSS Qt5-based projects have a single place to-go-to for
>> > nice charting. Which would be centrally updated now and then. Still not
>> > perfect, but an improvement over the current situation.
>> > To meet the KDE repo requirements, KDAB has also extended the license to
>> > GPLv2+ for this dump :)
>> Thanks KDABians for this.
>> If this is basically a copy&paste from the existing code we're already
>> shipping i have no objection,
> Yes, nearly copy&paste: the copies of KDChart in Calligra & KMyMoney are older
> (2.4.1, based on Qt4) versions, while the copy of KDChart in Massif-Visualizer
> matches the version of the KChart lib in KDiagram.
>> though you should probably get someone with
>> CMake knowledge to review (and kill that autogen.py and g++.pri?)
> Yes, any volunteers here for reviewing the CMake parts? :) Code should be
> similar to that of a KF5 tier1 lib.
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
- Library targets are usually called KF5*, see KF5Parts or KF5Gantt
- 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
- some of the definitions in the root CMakeLists.txt files are already
being defined by KDEFrameworkCompilerSettings. You might want to clean
- misses a .reviewboardrc file.
I created a small review request you also want to look into:
More information about the kde-core-devel