KDiagram libs (KChart, KGantt) in KDE Review

[Aleix Pol]

On Mon, Feb 9, 2015 at 11:35 AM, Inge Wallin <inge at lysator.liu.se> wrote:
> On Monday, February 09, 2015 04:09:59 Aleix Pol wrote:
>> 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
>> >> va
>> >>
>> >> escriure:
>> >> > 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.
>> >
>> > Cheers
>> > Friedrich
>>
>> Hi,
>> 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
>
> KDiagram is not a framework and cannot become one until the library is release
> under the LGPL license (right?). So a name starting with KF5 sounds strange to
> me at this point.
>
>> - 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.
>> - 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:
>> https://git.reviewboard.kde.org/r/122495/
>>
>> Aleix

Correct, my apologies, I was thinking it might become a framework.

Aleix