[Konversation-devel] Review Request: Make dependency on kdepim optional

[Bernd Buschinski]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103551/#review9590
-----------------------------------------------------------


Don't Ship It.

This Patch needs some cleanup, it currently doesn't even compile for systems with kdepimlibs.


CMakeLists.txt
- macro_optional_find_package + macro_log_feature should be used
  To show a kdepim in featurelog.
  This also allows kdepimlibs to be disabled even if kdepimlibs is installed with -DWITH_KdepimLibs=OFF

- I don't like the name "ADDRESSBOOK_INTEGRATION", it should start with HAVE_
  like HAVE_KDEPIMLIBS, as we do with HAVE_QCA


src/CMakeLists.txt
- code between line 190 and 201 should be indented


src/irc/nickinfo.cpp
- line 280 to 289, removing the "else" is not good.
  It is not necessary indeed, but it represents a logical connection


src/irc/nicksoneline.cpp
- old line 315-316
    // Get addressbook entry (if any) for the nick.
    KABC::Addressee addressee = nickInfo->getAddressee();
  MUST NOT be removed, they are necessary for compile, if ADDRESSBOOK_INTEGRATION is defined,
  compile currently fails

- line 743, does not belong to the topic of this patch, it needs a different commit
  Same for line 756 and 762

- line 780, 784, 788, 804, 836, 849, 855
  removing the "else" is wrong as it shows a logical connection

src/viewer/irccontextmenus.h
- line 57, if ADDRESSBOOK_INTEGRATION is not defined,
  "pedantic" compile will fail because of the trailing "," in enum


src/irc/server.cpp
- line 2896, if ADDRESSBOOK_INTEGRATION is not defined, we get the compile warning
  server.cpp:2896:6: warning: unused parameter 'nickInfo' [-Wunused-parameter]

- Bernd Buschinski


On Dec. 27, 2011, 12:04 a.m., Thomas Capricelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103551/
> -----------------------------------------------------------
> 
> (Updated Dec. 27, 2011, 12:04 a.m.)
> 
> 
> Review request for Konversation.
> 
> 
> Description
> -------
> 
> For people outthere like me that do not use KDE, the dependency on kdepim is a very huge one. We would very happily make the tradeoff of NOT being able to link nicknames to kde addressbook.
> 
> I guess you see the points, depending on kdepim brings two big problems
> * installation/compilation : depending on kdepim brings half of kde packages, kdepim package and hence ugliness such as nepomuk/akonadi.
> * at runtime, starting konversation, by the magic of dbus and KDE spaghetti runtime dependencies, it starts lot of unrelated KDE stuff daemons that takes both cpu and ram. I know this arg is denied by KDE zealot, but i stand on it.
> 
> So I did it. This patch allows compilation without kdepim. I've tried to keep the patch as minimal as possible of course.
> 
> I'm always on irc/freenode as 'orzel', and i'll be on #konversation for the next days.
> Also, I do have commit rights on KDE (not sure about konversation proper, i did no follow the git migration very closely)
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt c8df038 
>   src/CMakeLists.txt a7ba9f4 
>   src/application.cpp 038b403 
>   src/irc/nick.cpp 298eafb 
>   src/irc/nickinfo.h 20b8ef1 
>   src/irc/nickinfo.cpp e768d0d 
>   src/irc/nicksonline.h fbdd2cd 
>   src/irc/nicksonline.cpp 8c9a4b1 
>   src/irc/outputfilter.cpp a7701e7 
>   src/irc/query.h 3685a53 
>   src/irc/query.cpp 1a432d4 
>   src/irc/server.h 775cb7c 
>   src/irc/server.cpp 111131a 
>   src/irc/serverison.h 4500564 
>   src/irc/serverison.cpp f737b51 
>   src/konsolepanel.cpp 14b32cc 
>   src/main.cpp f2c7a22 
>   src/mainwindow.cpp 112dd39 
>   src/viewer/irccontextmenus.h 40756d1 
>   src/viewer/irccontextmenus.cpp b735843 
> 
> Diff: http://git.reviewboard.kde.org/r/103551/diff/diff
> 
> 
> Testing
> -------
> 
> cmake with kdepim installed and with kdepim not installed
> compiles tested with kdepim not installed: compiles, link and work fine
> 
> 
> Thanks,
> 
> Thomas Capricelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konversation-devel/attachments/20120105/915e4337/attachment.html>