Review Request: Add backlog to chat window

[Dominik Schmidt]

> On Oct. 4, 2011, 10:53 p.m., Daniele Elmo Domenichelli wrote:
> > lib/adium-theme-message-info.cpp, lines 129-135
> > <http://git.reviewboard.kde.org/r/102776/diff/1/?file=37870#file37870line129>
> >
> >     Are the comments here useful?

I'm not really sure if they are needed here, so they might be added again at some point, probably should replace them with something more meaningful.


> On Oct. 4, 2011, 10:53 p.m., Daniele Elmo Domenichelli wrote:
> > lib/chat-widget.cpp, line 238
> > <http://git.reviewboard.kde.org/r/102776/diff/1/?file=37873#file37873line238>
> >
> >     Is there a reason why don't you set "this" as parent?

Yes, because I forgot it ;-)


> On Oct. 4, 2011, 10:53 p.m., Daniele Elmo Domenichelli wrote:
> > lib/logmanager.cpp, line 69
> > <http://git.reviewboard.kde.org/r/102776/diff/1/?file=37877#file37877line69>
> >
> >     Perhaps here it's not necessary a warning here, the warning should be at build time...

I disagree here, it might be useful to know to the user why he can see backlog on one system but not on another


> On Oct. 4, 2011, 10:53 p.m., Daniele Elmo Domenichelli wrote:
> > lib/logmanager.cpp, line 127
> > <http://git.reviewboard.kde.org/r/102776/diff/1/?file=37877#file37877line127>
> >
> >     This probably shouldn't be a warning

Ack


> On Oct. 4, 2011, 10:53 p.m., Daniele Elmo Domenichelli wrote:
> > lib/logmanager.cpp, line 143
> > <http://git.reviewboard.kde.org/r/102776/diff/1/?file=37877#file37877line143>
> >
> >     I'm, not sure if we want to exit here

Of course not, already remove in my branch.


On Oct. 4, 2011, 10:53 p.m., Dominik Schmidt wrote:
> > A general thought... Is the logmanager useful for something if TELEPATHY_LOGGER_QT4_FOUND = false?
> > If not, I'd avoid completely building it from cmake, it will clean a lot the code from all the #ifdef you will have to add just a couple of #ifdef in chat-widget.

Nope, but it found it cleaner to keep it completely self contained.

Once we change to Nepomuk we will get rid of them anyways and i don't need to touch the chat-widget again.


- Dominik


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


On Oct. 4, 2011, 9:54 p.m., Dominik Schmidt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102776/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2011, 9:54 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> I'm sorry this patch got so massive, I spent 4 hours trying to split it up and do a proper review request with no success :-(
> 
> Anyways, you can take a look at my branch with more granular commits:
> kde:clones/telepathy-text-ui/dschmidt/telepathy-text-ui gomental
> 
> 
> Now to the patch:
> It adds backlog functionality (currently a hardcoded amount of messages) to the chat widget via telepathy-logger-qt4. I made it an optional dep cause that lib is not released yet and I want to replace it with logs retrieved from Nepomuk for 0.3.
> 
> All logic interacting with that lib is encapsulated in the LogManager class, so it will be easy to do that change.
> 
> 
> Diffs
> -----
> 
>   cmake/modules/FindGIO.cmake PRE-CREATION 
>   cmake/modules/FindGObject.cmake PRE-CREATION 
>   cmake/modules/FindQtGLib.cmake PRE-CREATION 
>   cmake/modules/FindTelepathyGlib.cmake PRE-CREATION 
>   cmake/modules/FindTelepathyLogger.cmake PRE-CREATION 
>   cmake/modules/FindTelepathyLoggerQt4.cmake PRE-CREATION 
>   config/appearance-config.cpp 895040b 
>   lib/CMakeLists.txt 801749e 
>   lib/adium-theme-message-info.h d90112c 
>   lib/adium-theme-message-info.cpp 545444d 
>   lib/adium-theme-view.cpp 89879c8 
>   lib/chat-widget.h 0fe5d86 
>   lib/chat-widget.cpp 302d097 
>   lib/chat-window-style.h 32fe4fd 
>   lib/chat-window-style.cpp 8242d90 
>   lib/logmanager.h PRE-CREATION 
>   lib/logmanager.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/102776/diff/diff
> 
> 
> Testing
> -------
> 
> Yes, compiled with -DWITH_TelepathyLoggerQt4=ON and -DWITH_TelepathyLoggerQt4=OFF and both builds work.
> 
> 
> Screenshots
> -----------
> 
> chat window with backlog
>   http://git.reviewboard.kde.org/r/102776/s/286/
> 
> 
> Thanks,
> 
> Dominik Schmidt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20111005/ed2a1ea8/attachment-0001.html>