----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101362/#review3322 -----------------------------------------------------------
main-widget.h <http://git.reviewboard.kde.org/r/101362/#comment2791> I don't think it is useful to keep a pointer to that label in the class members. main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2785> Misssing i18n: QLabel(i18n("No Accounts Found")); main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2786> i18n("No Accounts Found") main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2787> i18n("Configure Accounts") main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2788> Is this necessary? doesn't the dialog automatically set its size to be equal to the size hint? main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2790> okClicked() should also be connected to close(), so that this message dialog is closed after the kcm is shown. main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2789> It would also be useful to set the Qt::WA_DeleteOnClose attribute to the dialog, so that it is deleted after it is closed. m_noAccountsDialog->setAttribute(Qt::WA_DeleteOnClose) main-widget.cpp <http://git.reviewboard.kde.org/r/101362/#comment2784> Coding style: always use {} in if statements, even if there is only one line of code in them. Also, how about adding an else statement that deletes the dialog? It is not going to be needed, so why keep it in memory? - George On May 14, 2011, 3:34 p.m., Tarun Mall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101362/ > ----------------------------------------------------------- > > (Updated May 14, 2011, 3:34 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This patch will check if no accounts will be present then an dialog box will > be opened telling no accounts found and giving an option to create on. > > > Diffs > ----- > > main-widget.h 7a5e417 > main-widget.cpp 20e8003 > > Diff: http://git.reviewboard.kde.org/r/101362/diff > > > Testing > ------- > > I tested this patch with > 1. No accounts > 2. Creating one account > 3. Deleting that account and then creating one again. > > > Thanks, > > Tarun > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
