> On July 2, 2014, 2:20 p.m., David Edmundson wrote: > > TelepathyLoggerQt4/log-manager.h, line 210 > > <https://git.reviewboard.kde.org/r/119085/diff/1/?file=286653#file286653line210> > > > > why not just a bool? > > Marcin Ziemiński wrote: > If it fails false value would be misleading.
I don't see any use in the QVariant approach. If the tpl_log_manager_* method fails, it will return false, so the QVariant() will still be valid, instead of invalid. When the account is invalid, then false is a legitimate return value, because entity from an invalid account does not have logging disabled, so there's nothing misleading on it (same as passing in invalid or non-existent entity - the don't have logging disabled either, so TplLogManager would just return false). > On July 2, 2014, 2:20 p.m., David Edmundson wrote: > > TelepathyLoggerQt4/log-manager.cpp, line 148 > > <https://git.reviewboard.kde.org/r/119085/diff/1/?file=286654#file286654line148> > > > > Tp account isn't a great variable name, as they are both teleapthy > > accounts. > > > > Maybe tpl (for telepathy logger) > > > > though I see you're just copying convention here. Up to you. Yeah, it's not perfect, but I'm fine with this here. Better to have the same convention in the entire file. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119085/#review61476 ----------------------------------------------------------- On July 2, 2014, 1:59 p.m., Marcin Ziemiński wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119085/ > ----------------------------------------------------------- > > (Updated July 2, 2014, 1:59 p.m.) > > > Review request for Telepathy. > > > Repository: telepathy-logger-qt > > > Description > ------- > > Added simple wrappers of functions available in the master branch of > telepathy-logger > > > Diffs > ----- > > TelepathyLoggerQt4/log-manager.h 60a0e8c > TelepathyLoggerQt4/log-manager.cpp 7aadc4d > > Diff: https://git.reviewboard.kde.org/r/119085/diff/ > > > Testing > ------- > > > Thanks, > > Marcin Ziemiński > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
