----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/102023/#review4908 -----------------------------------------------------------
Ship it! account-button.h <http://git.reviewboard.kde.org/r/102023/#comment4270> .h files should include the least amount of things possible, it speeds up compiles - especially as this now includes, everything KMenu included. because you only store a pointer to a KMenu the compiler doesn't need to know how big it is, so we can just forward declare the class with class KMenu; Which you can see we've done for some other classes, such as KLineEdit. This only applies to storing/passing pointers to objects. You will then need to #include it in the cpp file. It's more of a good practice thing, rather than anything actually "wrong". account-button.cpp <http://git.reviewboard.kde.org/r/102023/#comment4271> You don't really need m_presenceMenu to be a member variable, it could be just local. If you want access this menu again, you can simply call menu(). Again, you've not anything wrong - just suggesting an alternative that is arguably a teeeeeny bit better and something to think about. - David On July 20, 2011, 5:05 p.m., Maxime Corteel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/102023/ > ----------------------------------------------------------- > > (Updated July 20, 2011, 5:05 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > Added a title to the account presence menu showing the account displayName > > > Diffs > ----- > > account-button.h 8dbf315 > account-button.cpp 5c9e135 > > Diff: http://git.reviewboard.kde.org/r/102023/diff > > > Testing > ------- > > Changed status > > > Screenshots > ----------- > > Menu with title > http://git.reviewboard.kde.org/r/102023/s/208/ > > > Thanks, > > Maxime > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
