----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116746/#review52696 -----------------------------------------------------------
OK, you're closer to the solution. I made some usual notes mainly related to coding standards and the APIs. kexi/plugins/reports/kexireportview.h <https://git.reviewboard.kde.org/r/116746/#comment37163> Coding style note: Please use camelCase here, so: openExportedDocument kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37167> This cannot be left untranslated, so i18n() is needed. But I would left defult QString() here. kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37168> In KDE standard actions (Yes/No/...) are defined once in KStandardGuiItem. Otherwise translators would translate "Yes" thousands of times again. But please carefully study http://community.kde.org/Kexi/Junior_Jobs/Implement_%22Do_you_want_to_open%22_question_after_exporting_report#Requirements -- I asked for using appropriate KStandardGuiItem - open and close, respectively. kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37165> coding standards note: empty line note needed kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37166> coding standards note: empty line note needed kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37164> KStandardGuiItem::close() is not a closing method, it's a data structure for denoting a close action. In fact the KMessageBox::questionyesNo() is synchronous and do not need closing, it closes before returning its value. So else{} can be left empty. kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37169> coding standard note: no comment needed for properly named method kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37170> coding standard note: no comment needed for properly named method kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37171> coding standard note: no comment needed for properly named method kexi/plugins/reports/kexireportview.cpp <https://git.reviewboard.kde.org/r/116746/#comment37172> 2 empty lines added by mistake plugins/videoshape/VideoThumbnailer.cpp <https://git.reviewboard.kde.org/r/116746/#comment37173> edit by mistake? - Jarosław Staniek On March 11, 2014, 10:27 p.m., Vishwa Modi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116746/ > ----------------------------------------------------------- > > (Updated March 11, 2014, 10:27 p.m.) > > > Review request for Calligra. > > > Repository: calligra > > > Description > ------- > > The patch implements "Do you want to open exported document?" question dialog > after exporting report to Text/Spreadsheet/as Web Page. > The private function "openexporteddocument(KUrl destination)" is defined in > file kexireportview.cpp which implements the task. > > > Diffs > ----- > > kexi/plugins/reports/kexireportview.h 4d7d017 > kexi/plugins/reports/kexireportview.cpp 3e0d709 > plugins/videoshape/VideoThumbnailer.cpp ddb5fb7 > > Diff: https://git.reviewboard.kde.org/r/116746/diff/ > > > Testing > ------- > > > Thanks, > > Vishwa Modi > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
