Review Request 109500: Refactor of the savesystem

[Aaron J. Seigo]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130322/73df6996/attachment-0001.html>