----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119631/#review63907 -----------------------------------------------------------
src/CMakeLists.txt <https://git.reviewboard.kde.org/r/119631/#comment44566> Could you please change the directory name to QML? It's a lot more descriptive than "import" src/imports/CMakeLists.txt <https://git.reviewboard.kde.org/r/119631/#comment44567> These first 5 lines aren't required. You can add the additional dependencies to the root CMakeLists.txt. Why do we need KDeclarative? src/imports/CMakeLists.txt <https://git.reviewboard.kde.org/r/119631/#comment44568> This is not required. src/imports/CMakeLists.txt <https://git.reviewboard.kde.org/r/119631/#comment44569> ditto src/imports/CMakeLists.txt <https://git.reviewboard.kde.org/r/119631/#comment44570> I could be wrong, but I don't think you're using QtQuick of KF5::Declarative. src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44577> This constructor is not really required. It's not a copy constructor since it has an extra "QObject* parent" Plus, since we do not have a d pointer, the default copy constructor will work for us. src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44579> The operator = and operator == are not required. src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44575> How about you just use Baloo::Result, instead of creating a BalooEntry class? src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44571> What is this function used for? src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44572> The IconRole is not requried, we can use the decoration role. src/imports/baloodatamodel.h <https://git.reviewboard.kde.org/r/119631/#comment44573> The TextRole is not required. We can use the Qt::DisplayRole. src/imports/baloodatamodel.cpp <https://git.reviewboard.kde.org/r/119631/#comment44578> #include "query.h" src/imports/baloodatamodel.cpp <https://git.reviewboard.kde.org/r/119631/#comment44576> Why 25? src/imports/baloodatamodel.cpp <https://git.reviewboard.kde.org/r/119631/#comment44580> You take the query, but you do not take ownership of the parent, so it will never be deleted. Maybe you can explicitly delete it in the destructor? src/imports/balooplugin.cpp <https://git.reviewboard.kde.org/r/119631/#comment44581> Oh. I see you do use KDeclarative, but it's not really required as we do not have any strings that need to be translated. Perhaps you could remove this? src/imports/balooplugin.cpp <https://git.reviewboard.kde.org/r/119631/#comment44582> Not really required. Automoc will take care of it. - Vishesh Handa On Aug. 6, 2014, 8:32 a.m., Antonis Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119631/ > ----------------------------------------------------------- > > (Updated Aug. 6, 2014, 8:32 a.m.) > > > Review request for Baloo. > > > Repository: baloo > > > Description > ------- > > This component exports some of the basic API of the Baloo::Query into to QML. > > At the moment it supports only the Baloo::Query::setType and > Baloo::Query::setLimit, > with the following syntax. > > BalooModel { > query { > type: "File/Image" > limit: 10 > } > } > > > Diffs > ----- > > src/imports/baloodatamodel.cpp PRE-CREATION > src/imports/balooplugin.h PRE-CREATION > src/imports/balooplugin.cpp PRE-CREATION > src/imports/qmldir PRE-CREATION > src/CMakeLists.txt 810f6dc > src/imports/CMakeLists.txt PRE-CREATION > src/imports/baloodatamodel.h PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/119631/diff/ > > > Testing > ------- > > > Thanks, > > Antonis Tsiapaliokas > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
