Review Request: Fix bug 276082 and refactor timeline in plasmate

Etienne Rebetez etienne.rebetez at oberwallis.ch
Thu Jun 23 20:29:45 CEST 2011



> On June 23, 2011, 9:24 a.m., Aaron J. Seigo wrote:
> > thanks for the patch. i believe it needs some work before it can go in, however.
> > 
> > first off: where is the bug fix, precisely? the patch is large and the bug fix is evidently somewhere in there.
> > 
> > after that, there are some issues i have with the refactor related to stability, speed and code readability.
> > 
> > finally, at most trivially, there remain numerous whitespace issues. this file was already pretty bad, we don't need to make it even more inconsistent, and fully bringing it into line with kdelibs style would be nice.
> > 
> > as for devel policy, doing such things in branches is definitely preferred. due to the size of the plasmate project (small) we do allow/expect people to merge their own branches into master when they are Ready(tm).

Thank you very much for our review:)
I'll try to implement your suggestions.


- Etienne


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


On June 22, 2011, 7:37 p.m., Etienne Rebetez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101728/
> -----------------------------------------------------------
> 
> (Updated June 22, 2011, 7:37 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Hi,
> 
> When trying out plasmate the last day, i was hit with the bug 276082. Since i didn't want that it happens to someone else, i made a fix for it.
> I the process i found that the loadTimeLine function was far to big. So i did also some refactoring. (this diff is kinda huge but i made the changes in a branch which i can merge to master)
> The timeliene and savepoint creation should now behave as expected.
> 
> Let me know what you think.
> Etienne
> 
> PS: How is the commit policy in plasmate? Could i just push directly to master in the future?
> 
> 
> Diffs
> -----
> 
>   mainwindow.cpp 8326a29 
>   savesystem/timeline.h 719fb73 
>   savesystem/timeline.cpp d258a1a 
>   savesystem/timelineitem.h 1e9699f 
>   savesystem/timelineitem.cpp d9b0a67 
> 
> Diff: http://git.reviewboard.kde.org/r/101728/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Etienne
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110623/5548e171/attachment-0001.htm 


More information about the Plasma-devel mailing list