Integrate Alkimia
Needs ReviewPublic

Authored by wojnilowicz on Jul 31 2018, 6:10 PM.

Details

Reviewers
tbaumgart
habacker
Group Reviewers
KMyMoney
Summary

Christian gave rationale for this move here

This is 1:1 move over of alkimia. All things that cause us compilation warnings will be mended latter on.

Test Plan

All tests pass.

Diff Detail

Repository
R261 KMyMoney
Lint
Lint Skipped
Unit
Unit Tests Skipped
wojnilowicz requested review of this revision.Jul 31 2018, 6:10 PM
wojnilowicz created this revision.

I'm at least somewhat against this change. Maybe it's just wishful thinking, but if we improve Alkimia, maybe there will be other users. That becomes essentially impossible if the code is moved internal to KMM.

Note also Ralf's comment in https://bugs.kde.org/show_bug.cgi?id=396901#c3. That seems to me that Skrooge is also using at least some aspect of alkimia.

Christian David mentioned at https://mail.kde.org/pipermail/kmymoney-devel/2018-May/020846.html to wait until 23rd of October 2018 before merging alkimia in case no feature has been added.

Christian David mentioned at https://mail.kde.org/pipermail/kmymoney-devel/2018-May/020846.html to wait until 23rd of October 2018 before merging alkimia in case no feature has been added.

Who's going to add new features?
That is Christian's point of view. I'm not going to sit and wait longer for a miracle to happen. Nobody develops alkimia and it's unhandy to fix compiler warnings with it being in a separate library.

I'm at least somewhat against this change. Maybe it's just wishful thinking, but if we improve Alkimia, maybe there will be other users. That becomes essentially impossible if the code is moved internal to KMM.

Note also Ralf's comment in https://bugs.kde.org/show_bug.cgi?id=396901#c3. That seems to me that Skrooge is also using at least some aspect of alkimia.

You can start to improve alkimia by yourself and then we may switch back to it, ok?
You are spreading wrong information. Skrooge doesn't use alkimia at all.

Christian David mentioned at https://mail.kde.org/pipermail/kmymoney-devel/2018-May/020846.html to wait until 23rd of October 2018 before merging alkimia in case no feature has been added.

Who's going to add new features?
That is Christian's point of view. I'm not going to sit and wait longer for a miracle to happen. Nobody develops alkimia and it's unhandy to fix compiler warnings with it being in a separate library.

It is also my POV. Alkimia is stable and does its job. At this point, I don't care for compiler warnings. We certainly have more severe problems than compiler warnings which might have been introduced recently and exist more or less on Windows environments only. I don't want to jeopardize the current stability by changing one of the core elements of KMyMoney. That said, I am against this change to be applied to the current master or 5.0 branch at least until the next KMyMoney release.

I'm at least somewhat against this change. Maybe it's just wishful thinking, but if we improve Alkimia, maybe there will be other users. That becomes essentially impossible if the code is moved internal to KMM.

Note also Ralf's comment in https://bugs.kde.org/show_bug.cgi?id=396901#c3. That seems to me that Skrooge is also using at least some aspect of alkimia.

You can start to improve alkimia by yourself and then we may switch back to it, ok?

In case changes are necessary to alkimia to solve the warning issues we are certainly in a position to make them and release alkimia just as we have done in the past. As usual: patches are welcome.

Christian David mentioned at https://mail.kde.org/pipermail/kmymoney-devel/2018-May/020846.html to wait until 23rd of October 2018 before merging alkimia in case no feature has been added.

Who's going to add new features?
That is Christian's point of view. I'm not going to sit and wait longer for a miracle to happen. Nobody develops alkimia and it's unhandy to fix compiler warnings with it being in a separate library.

It is also my POV. Alkimia is stable and does its job. At this point, I don't care for compiler warnings. We certainly have more severe problems than compiler warnings which might have been introduced recently and exist more or less on Windows environments only. I don't want to jeopardize the current stability by changing one of the core elements of KMyMoney. That said, I am against this change to be applied to the current master or 5.0 branch at least until the next KMyMoney release.

So let's make a release and integrate alkimia after that.

Windows version of KMyMoney wasn't available until recently, so I don't jeopardize anything but try to make it work decently on that platform.
Withouth this "core element" nothing in KMyMoney would work right after the move over and it does on the contrary. Every test depends on it and passes.

I'm at least somewhat against this change. Maybe it's just wishful thinking, but if we improve Alkimia, maybe there will be other users. That becomes essentially impossible if the code is moved internal to KMM.

Note also Ralf's comment in https://bugs.kde.org/show_bug.cgi?id=396901#c3. That seems to me that Skrooge is also using at least some aspect of alkimia.

You can start to improve alkimia by yourself and then we may switch back to it, ok?

In case changes are necessary to alkimia to solve the warning issues we are certainly in a position to make them and release alkimia just as we have done in the past. As usual: patches are welcome.

It's unhandy to do that. Let's face it. That library is dead. Nobody besides us use it and nobody develops it since long time and I think it's unwise to force otherwise. In my opinion it's not practical to have it external.

I rebased the patch to fit the latest master branch.
Please test and approve.

There are several people who are against this, and only one in favor. Would you please consider others' opinions, and defer this further, or put it in a separate branch?

There are several people who are against this, and only one in favor. Would you please consider others' opinions, and defer this further, or put it in a separate branch?

No.