> On Nov. 26, 2012, 1:43 p.m., Diego Casella wrote: > > plasmate/CMakeLists.txt, line 102 > > <http://git.reviewboard.kde.org/r/107470/diff/1/?file=96184#file96184line102> > > > > I'm wondering if we really need to add a kdepimutils dependency for a > > simple email validation... Thoughts about that? > > > > In any case, "kpimutils" must be discarded and substituted with the > > more portable ${KDEPIMLIBS_KPIMUTILS_LIBS} (more info inside > > FindKdepimLibs.cmake file). > > Giorgos Tsiapaliokas wrote: > >I'm wondering if we really need to add a kdepimutils dependency for a > simple email validation... Thoughts about that? > Plasmate already depends on qgpgme++ library which is in kdepimlibs. > Kdepimutils are also part of the kdepimlibs. > So we don't introduce a new depedency for a new repository. > > Also we have been advised for this change from the kdereview. > > Antonis Tsiapaliokas wrote: > As regards your 2nd statement, KPIMUtils::isValidSimpleAddress doesn't > have a correct api, so that was the best thing that i found. But yes, that > could be changed. > > Now as regards the depedency, i agree, that we don't really need that > library. But the people on kcd, said that we should replace it. >
This is the right answer, sorry again for the noise. >>I'm wondering if we really need to add a kdepimutils dependency for a simple >>email validation... Thoughts about that? >Plasmate already depends on qgpgme++ library which is in kdepimlibs. >Kdepimutils are also part of the kdepimlibs. Actually, I made Plasmate to depend on QGpgme _only_ (that is, libqgpgme.so), not to kdepimlibs/kpimutils, in order to avoid unnecessary library inclusion/linkage dependencies. In fact, it looks for package "QGpgme" using FindQGpgme.cmake and links against ${QGPGME_LIBRARIES}, instead of retrieving it through the more general FindKdepimLibs.cmake. So, using the method in this patch will add a new library dependency to Plasmate (libkpimutils.so), that's it. I still don't like the idea to increase the number of required libs because of one simple line of code; anyway, since we are in democracy, and kcd guys told you that it's fine for them to do that, I won't object anymore about that point. Feel free to commit, but remember to fix the cmake file first ;) - Diego ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107470/#review22561 ----------------------------------------------------------- On Nov. 26, 2012, 10:36 a.m., Antonis Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107470/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2012, 10:36 a.m.) > > > Review request for Plasma. > > > Description > ------- > > On this patch i am replace the QRegExp on SigningDialog::validateParams() > with KPIMUtils::isValidSimpleAddress > > > Diffs > ----- > > plasmate/CMakeLists.txt 111c402 > plasmate/publisher/signingdialog.cpp 395138d > > Diff: http://git.reviewboard.kde.org/r/107470/diff/ > > > Testing > ------- > > > Thanks, > > Antonis Tsiapaliokas > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel