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

Ship it!


Looking at the code, it seems apparent that DvcsJob could use some re-factoring 
as well. It is a KJob .. but it doesn't behave like one at all, nor is it used 
like one. Either it should be made a properly async KJob, or if that is neither 
needed nor desired then it should probably not subclass from KJob. Note that it 
is usually new'd and then deleted all within the same method (or methods it 
calls) which says to me that it isn't async right now and probably should just 
be created on the stack and passed around ..

This is something for a different patch set of course :)

- Aaron J. Seigo


On March 20, 2013, 6:16 p.m., Giorgos Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109500/
> -----------------------------------------------------------
> 
> (Updated March 20, 2013, 6:16 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Hello,
> 
> this patch is the refactor of the savesystem.
> 
> Also in this refactor the above bugs are being fixed.
> 
> Q: Why do we need a refactor?
> A: * Before this patch in order to take the git log we did
> some parsing, which wasn't nice. With this patch we use git log 
> --pretty-format.
> * There are some other changes like coding style stuff and striping \n from 
> strings.
> 
> 
> This addresses bugs 316202, 316724 and 316725.
>     http://bugs.kde.org/show_bug.cgi?id=316202
>     http://bugs.kde.org/show_bug.cgi?id=316724
>     http://bugs.kde.org/show_bug.cgi?id=316725
> 
> 
> Diffs
> -----
> 
>   plasmate/savesystem/dvcsjob.cpp 288d7a6 
>   plasmate/savesystem/gitrunner.h 50f87ab 
>   plasmate/savesystem/gitrunner.cpp b263be6 
>   plasmate/savesystem/timeline.h 73849d0 
>   plasmate/savesystem/timeline.cpp 231fc46 
> 
> Diff: http://git.reviewboard.kde.org/r/109500/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Giorgos Tsiapaliokas
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to