> On July 4, 2011, 8:48 a.m., Vishesh Handa wrote:
> > nepomuk/services/strigi/nepomukindexer.h, line 50
> > <http://git.reviewboard.kde.org/r/101790/diff/2/?file=25638#file25638line50>
> >
> >     I've would prefer something like IndexingJob, which makes it obvious it 
> > is an asynchronous process implemented via KJob.
> 
> Paweł Paprota wrote:
>     I think more important is that it launches nepomukindexer binary using 
> KProcess but if you insist I can change it.

Actually it launching the indexer process is an implementation detail. Thus, it 
is bad design to put that into the name of the class. Vishesh is correct in 
suggesting a name like IndexingJob.


> On July 4, 2011, 8:48 a.m., Vishesh Handa wrote:
> > nepomuk/CMakeLists.txt, line 10
> > <http://git.reviewboard.kde.org/r/101790/diff/2/?file=25636#file25636line10>
> >
> >     I don't think contaminating the global CMakeLists.txt is a good idea. 
> > Whatever includes are required should be in the local CMakeLists.txt
> 
> Paweł Paprota wrote:
>     IIRC, those includes are needed for various different source folders, 
> that's why I moved it to the root. In general include paths are a mess. I 
> will revisit this and try to move them to more specific places...

Please try to refrain from using expressions like "are a mess" when talking 
about work others have done.


- Sebastian


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


On June 28, 2011, 7:31 a.m., Paweł Paprota wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101790/
> -----------------------------------------------------------
> 
> (Updated June 28, 2011, 7:31 a.m.)
> 
> 
> Review request for Nepomuk.
> 
> 
> Summary
> -------
> 
> This is a little ground work for writing integration tests in order to test 
> Nepomuk components in the "real" setup - with D-Bus/socket communication, 
> Virtuoso repository, KConfig configuration running etc.
> 
> Also included are simple tests for indexing of files (see indexertests.cpp).
> 
> See README for details.
> 
> The code is also available here:
> 
> https://github.com/ppawel/kde-runtime/tree/integration-tests
> 
> 
> Diffs
> -----
> 
>   nepomuk/CMakeLists.txt c6a1879 
>   nepomuk/services/strigi/indexscheduler.cpp 56914a7 
>   nepomuk/services/strigi/nepomukindexer.h 08abb2a 
>   nepomuk/services/strigi/nepomukindexer.cpp d796983 
>   nepomuk/services/strigi/strigiservice.h 2eb7eac 
>   nepomuk/services/strigi/strigiservice.cpp 0189679 
>   nepomuk/tests/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/README PRE-CREATION 
>   nepomuk/tests/indexertests.h PRE-CREATION 
>   nepomuk/tests/indexertests.cpp PRE-CREATION 
>   nepomuk/tests/lib/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/lib/NepomukTestLibMacros.cmake PRE-CREATION 
>   nepomuk/tests/lib/nepomuk-repository-with-ontologies.tgz PRE-CREATION 
>   nepomuk/tests/lib/nepomukserverrc.in PRE-CREATION 
>   nepomuk/tests/lib/nepomuktest_export.h PRE-CREATION 
>   nepomuk/tests/lib/testbase.h PRE-CREATION 
>   nepomuk/tests/lib/testbase.cpp PRE-CREATION 
>   nepomuk/tests/runNepomukTest.sh PRE-CREATION 
>   nepomuk/tests/test/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/test/filewatch/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/test/filewatch/filewatch.h PRE-CREATION 
>   nepomuk/tests/test/filewatch/filewatch.cpp PRE-CREATION 
>   nepomuk/tests/test/identificationtest.h PRE-CREATION 
>   nepomuk/tests/test/identificationtest.cpp PRE-CREATION 
>   nepomuk/tests/testdata/CMakeLists.txt PRE-CREATION 
>   nepomuk/tests/testdata/klogo.png PRE-CREATION 
>   nepomuk/tests/testdata/trollface.jpg PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101790/diff
> 
> 
> Testing
> -------
> 
> Tests should execute with simple "make test". I did however have some 
> problems (segfaults) with sendEvents method that notifies the user that 
> "indexing started for fast search" - I had to disable it locally, I will try 
> to track down the cause of this if it is reproducible.
> 
> 
> Thanks,
> 
> Paweł
> 
>

_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk

Reply via email to