Ok, I pushed some changes to:
* clean the BookmarkModel d-pointer issues * rename Contact "d" to d_ptr * Remove the legacyhistorybackend as import from 2 years old version isn't important anymore * Fix the cmakelist duplicated line Missing d-pointers: * The ringtone model, a new version is in a github branch, I have yet to review and merge it * SecurityEvaluationModel is currently being rewriten, fixing it now is pointless, the new version wont be ready before this review expire All other classes without d-pointer are interfaces or proxy models, they don't need d-pointers. Regards, Emmanuel Lepage On 14 January 2015 at 18:22, Elv1313 . <elv1...@gmail.com> wrote: > Hello Albert, > > Thanks for your comments. > >> * src/abstractitembackend.h is twice in libringclient_LIB_HDRS > > I will fix that, thanks, maybe we should add a Krazy2 check for that. > I know Laurent Model has posted one on planetkde.org for the code > a while back, I haven’t used it yet. > >> * You have some d-pointers in some of your classes but not on others, why is > that? > > Umm, I was under the impression I had fixed that in the commit before posting > this email. Krazy2 doesn't seem to analyze LibRingClient yet, even if I > checked it on projects.kde.org. Maybe it does really take more than 5 days? > Anyhow, I will fix more of them. Some classes are really just templates > aliases > + QObject inheritance, so those obviously don't need d-pointers. Other simply > have no private members, I am not sure if I should add d-pointer to those just > in case, maybe I should. > >> * There's a catch regarding translations. You are using >> $XGETTEXT_QT in your Messages.sh. That creates a .po file that only works for >> clients that access those translations via kdelibs4 i18n() (or maybe other >> gettext wrappers) but not (i think) via kf5 ki18n() or plain Qt tr(). For Qt5 >> we have $EXTRACT_TR_STRINGS that creates a proper .tr file to use with Qt5 >> (no >> idea if it works with Qt4, maybe it does). This is a bit weird too because >> your branch has both Qt4 and Qt5 support in the same branch but our >> translation system is build around the expectation that there will be >> separate >> branches and thus they will have different Messages.sh catering for each qt4 >> and qt5. At this stage I'm not sure what's the best thing for you guys to do >> :/ Are you mainly targetting qt4 or qt5 for clients to use? > > The problem is that we are doing a very major release right now and the kf5 > port > is not ready yet, so it wont make the cut for our deadlines. There is 2 or 3 > other projects trying to release a fully decentralized and secure > communication > playform right now. We think we still are the closest to that, so we hurry up. > I prefer to spend my time on security details than KF5 port for the next > month, > so we will miss the spring distribution cycle for KF5. After the 2.0 release, > getting rid of the Qt4 support will be something I will do. I don't care about > Qt4 at this point, but the KDE ui is still using KDE4 and I also wait for > KDEPim > kf5 support to mature before doing the switch. The others clients, such as GTK > and OS X, use Qt5 and some patches are starting to be Qt5 only, but I don't > want to drop Qt4 until about mid-March. I guess translation support that works > on KDE4 is better than nothing for now. The new OSX and Gnome client don't > have > any translations yet anyway, so the fact that the few tr() in libringclient > are > not translated don't have an impact on them yet. > > As you saw, it also cause the CMakeLists.txt to be ugly and bloated and I also > have to keep the deprecated/qt4support flag on. I also can't use the abstract > proxy model and other classes that made their way into QtCore, nor switch to > the much easier to lint connect() syntax. There is quite a few //TODO qt5 in > the code. This is a (temporary) very bad situation... > > Regards, > Emmanuel
