> 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
