[Nepomuk] Re: Review Request: WritebackJob draft

[Sebastian Trueg]

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



nepomuk/services/writeback/service/writebackjob.cpp
<http://git.reviewboard.kde.org/r/102094/#comment4654>

    finished(this) is wrong. You need the signal signature which involves the type of the parameter, ie.
    SIGNAL(finished(Nepomuk::WritebackPlugin*))
    
    And btw: signals and slots should always be fully qualified, ie. including the full namespace.
    
    Maybe you should test your code? ;)



nepomuk/services/writeback/service/writebackservice.cpp
<http://git.reviewboard.kde.org/r/102094/#comment4655>

    Please combine the next line into this one to not have an undefined pointer lying around, even if only for one line.
    
    If you get used to never do that you avoid errors down the line.



nepomuk/services/writeback/service/writebackservice.cpp
<http://git.reviewboard.kde.org/r/102094/#comment4656>

    Why don't you put all this into a separate method which takes as parameter a KService::List. Then you avoide code duplication.


- Sebastian


On July 27, 2011, 7:24 p.m., Smit Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102094/
> -----------------------------------------------------------
> 
> (Updated July 27, 2011, 7:24 p.m.)
> 
> 
> Review request for Nepomuk.
> 
> 
> Summary
> -------
> 
> This is my first attempt to write the writebackjob for the writebackservice.
> 
> 
> Diffs
> -----
> 
>   nepomuk/services/writeback/lib/writebackplugin.h 6bcdfd3 
>   nepomuk/services/writeback/lib/writebackplugin.cpp 2a52a31 
>   nepomuk/services/writeback/service/CMakeLists.txt 949b379 
>   nepomuk/services/writeback/service/writebackjob.h e69de29 
>   nepomuk/services/writeback/service/writebackjob.cpp e69de29 
>   nepomuk/services/writeback/service/writebackservice.h a9821a6 
>   nepomuk/services/writeback/service/writebackservice.cpp be5c738 
> 
> Diff: http://git.reviewboard.kde.org/r/102094/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Smit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20110728/e5eb4387/attachment.htm