Review Request: Nepomuk desktop query API

Sebastian Trueg trueg at kde.org
Sat Nov 7 09:14:23 GMT 2009 


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/dateparser.h, line 39
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13751#file13751line39>
> >
> >     Should be hidden in private class

This class is supposed to be hidden actually. I will thus, rename the header to dataparser_p.h


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/dateparser.h, line 46
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13751#file13751line46>
> >
> >     Why not using enum with QFLAGS here?

No reason. Will fix it.


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/query.h, line 140
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13771#file13771line140>
> >
> >     All implementations of this class should got to the .cpp file. And a d-pointer is missing.

Agreed.


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/query.h, line 195
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13771#file13771line195>
> >
> >     Does it make sense to have these 'search file' specific methods in the general query object? Shouldn't there be a FileQuery class instead that inherits from Query?

That is a nice idea. I will have a look if that is easily doable.


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/query.h, line 247
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13771#file13771line247>
> >
> >     As it doesn't change the 'this' object but returns a new one, the method should be renamed to 'optimized', like QString::trimmed()

agreed.


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/querybuilderdata.h, line 1
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13774#file13774line1>
> >
> >     If this class is only used internally, rename it to querybuilderdata_p.h?

yes.


> On 2009-11-06 13:53:51, Tobias Koenig wrote:
> > trunk/KDE/kdelibs/nepomuk/query/queryparser.h, line 92
> > <http://reviewboard.kde.org/r/2061/diff/1/?file=13775#file13775line92>
> >
> >     Can be made const as well, or?

agreed.


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2061/#review2948
-----------------------------------------------------------


On 2009-11-04 14:54:51, Sebastian Trueg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2061/
> -----------------------------------------------------------
> 
> (Updated 2009-11-04 14:54:51)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> One thing has been missing ever since KDE 4.0: A good Nepomuk query API. We cannot expect app developers to all learn SPARQL.
> This API is the final version (Virtuoso-extensions-based) of the query API that has already been in kdebase/workspace/libs/nepomukquery and kdebase/runtime/nepomuk/libnepomukquery. The API is already used by the Nepomuk query service, the Nepomuk search kio slave, the Nepomuk Plasma runner, and the simple search client in playground.
> 
> It makes perfect sense to include this API in KDE 4.4.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/includes/CMakeLists.txt 1044143 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/AndTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/ComparisonTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/GroupTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/LiteralTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/NegationTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/OrTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/Query PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/QueryParser PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/QueryServiceClient PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/ResourceTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/Result PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/SimpleTerm PRE-CREATION 
>   trunk/KDE/kdelibs/includes/Nepomuk/Query/Term PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/CMakeLists.txt 1044143 
>   trunk/KDE/kdelibs/nepomuk/query/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/andterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/andterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/andterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/comparisonterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/comparisonterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/comparisonterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/dateparser.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/dateparser.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/dbusoperators.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/dbusoperators.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/groupterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/groupterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/groupterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/literalterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/literalterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/literalterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/negationterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/negationterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/negationterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/nepomukquery_export.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/nie.trig PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/org.kde.nepomuk.Query.xml PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/org.kde.nepomuk.QueryService.xml PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/orterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/orterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/orterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/query.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/query.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/query_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/querybuilderdata.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/queryparser.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/queryparser.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/queryserviceclient.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/queryserviceclient.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/resourceterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/resourceterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/resourceterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/result.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/result.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/simpleterm.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/simpleterm.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/simpleterm_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/term.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/term.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/term_p.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/pimo.trig PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/queryparsertest.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/queryparsertest.cpp PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/querytest.h PRE-CREATION 
>   trunk/KDE/kdelibs/nepomuk/query/test/querytest.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/2061/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>

More information about the kde-core-devel mailing list