SFLPhone-KDE is now Ring-KDE and is moving to kdereview
elv1313 at gmail.com
Wed Jan 14 23:22:14 GMT 2015
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
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...
More information about the kde-core-devel