> On July 11, 2014, 7:58 p.m., Christian David wrote: > > Is this only used until the pim libs are KF5 ready? Because I think this > > function is so important that it justifies having the pim libs a obligatory > > dependency. There will be many bug reports if a package maintainer forgets > > to compile with kdepimlibs. > > Cristian Oneț wrote: > No, I would like to see this as a permanent change. I'm not that sure > about which of the features are so important that they must be a mandatory > dependency. I like the idea of having the freedom to build without kdepimlibs > if necessary and if that can be obtained easily why not have it? I'm not > sugesting that packagers should do this but for example in source based > distributions users would be able to have a choice, which is nice. > > Alvaro Soliverez wrote: > I'm against this being a permanent change. It affects how schedules are > handled, which impacts all over the application. > And I certainly don't want to track down a bug reported by an unsupecting > user just to figure out later a packager left the dependency out because kde > pim libs was "too big". > > Cristian Oneț wrote: > I hate to disagree but I don't see it that way. The only effect to the > way schedules are handled is that processing dates are not obtained from the > holliday region thus the "old" (and documented) behavior of considering only > weekends as non-processing days will be in effect. > > If you ask me is this feature really that crucial that we should make > everyone build kdepimlibs in oder to use kmymoney I would say no. Again, I'm > not removing the feature, just making it optional. > > The feature isn't even documented so I'm not sure if we would start > receiving bug reports about it. But just to be on the safe side we could > finish the feature in main.cpp:69 which would provide a list of optional > features that were built (or found as plugins) in the about data. > > If it's something I've learned from and appreacited in the KF5 model is > that the take everything or leave it approach is wrong. Think about it, > kdepimlibs has 22 libraries plus their dependecies , of which we only use 4 > (QGpgme, KHolidays, PIMIdentities, Akonadi) for pretty slim features. Knowing > this wouldn't it be wiser to allow those who don't need those features to be > able to use the application without these dependencies? > > Marko Käning wrote: > Cristian, I am very much in favour for what you're stating here. This is > something which one wouldn't be too worried about on Linux, but on OSX as on > Windows in your case it is a real show-stopper to be forced into having far > too many dependencies for seemingly little functionality gain. > > Thus, I also vote for making things optional! > > Christian David wrote: > I think, on the long term kdepimlibs will be split up anyway. So the size > problem might be solved just by waiting. > > Please see my comments as not mainly targeting this review request but > more as a general concern about making kdepimlibs optional. > > From my point of view this patch adds more complexity to KMyMoney without > a real benefit. > > All future development needs to wrap the usage of kdepimlibs, which means > a lot of work. Also it is hard for a new developer to understand/change it as > he has to learn kdepimlibs and the KMyMoney wrapper. Also using #if > HAVE_KDEPIMLIBS is uncomfortable from my point of view but maybe I am just > too sensitive to that (I worked in project which overdid this — bad memories). > > If this has a real benefit for Windows or Mac users it could be a fair > deal until kdepimlibs got actually split up. Especially as long as it is a > small patch like this one. > > Cristian Oneț wrote: > I agree that preprocessor definition based conditonal compiling is a > PITA. KMyMoney already has KMM_DESIGNER which gave us a lot of headaches but > in this case I think that there is a real benefit as long as the conditional > code can be kept fairly simple and future development is not hindered. > > Cristian Oneț wrote: > This weekend I checked out the state of kde framemeworks 5 on Windows. > Once I passed the hurdles and managed to install all the necessary frameworks > (by fixing some issues) I would have found it much easier to start working on > KMyMoney, if this patch would have been in master and frameworrks. Knowing > the state of framworks kdepimlibs on Linux I didn't even tried to build it on > Windows, just went ahead and ifdef-ed the kdepimlibs dependencies so I could > get to the more important issue of KMyMoney on frameworks on Windows. > > I guess the situation is similar on OS X. The sooner we would be able to > setup a CI build of KMyMoney on all three systems (Linux, Windows, OS X) the > better because KMyMoney could act as a real proof that frameworks are really > cross platform like Qt.
That's not a reason to have this as a permanent change. We found the same kind of issues during our port to Qt4, and these will be fixed too. If this is going to go in, I'd like a very clear notice in the UI of the features disabled when the dependency is not included. No silent disabling. - Alvaro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119234/#review62153 ----------------------------------------------------------- On July 11, 2014, 7:58 p.m., Cristian Oneț wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119234/ > ----------------------------------------------------------- > > (Updated July 11, 2014, 7:58 p.m.) > > > Review request for KMymoney. > > > Repository: kmymoney > > > Description > ------- > > If KDE PIM libraries will not be found during configuration all days will be > considered processing days. Note that this review request dependes on the > cmake changes in https://git.reviewboard.kde.org/r/119207/ > > > Diffs > ----- > > kmymoney/dialogs/settings/ksettingsschedules.cpp 96bfb84 > kmymoney/kmymoney.cpp 687187a > > Diff: https://git.reviewboard.kde.org/r/119234/diff/ > > > Testing > ------- > > Build and run the application with and without HAVE_KDEPIMLIBS defined. > > > Thanks, > > Cristian Oneț > >
_______________________________________________ KMyMoney-devel mailing list KMyMoney-devel@kde.org https://mail.kde.org/mailman/listinfo/kmymoney-devel
