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


The first patch, for the bug, IMHO is better done in another way, so please 
update it.

And the second patch might result in errors perhaps (no idea really, don't know 
the code enough yet), so unless that is a real performance killer I would 
prefer to have that just a TODO comment for now.


plan/libs/kernel/kpttask.cpp
<http://git.reviewboard.kde.org/r/111657/#comment26912>

    Not sure CompletionActualEffort is the proper enum. Sadly they are not 
really documented, and then they seem to all make no difference anyway.
    
    So instead of reusing that one just add another item to enum 
Node::Properties, named UsedEffort, and use this as value for the changed(...) 
call.  And make it also results in clearing all clearPerformanceCaches, like 
the other enum values in Node::changed(Node *node, int property).



plan/libs/models/kptnodechartmodel.cpp
<http://git.reviewboard.kde.org/r/111657/#comment26911>

    Is there really only the projectNode which is without parents? I am not yet 
sure if there cannot be other nodes which would meet the condition.
    
    Especially as the whole logic might be there for a reason, so I would be 
rather cautious to change this.
    
    Perhaps just add a TODO for now, and once you feel really secure that this 
is the right optimization, do it.


- Friedrich W. H. Kossebau


On July 23, 2013, 1 p.m., Alvaro Soliverez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111657/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 1 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> * When effort is added to a task, use the Node::CompletionActualEffort 
> property to have the performance cache should be refreshed when it hits 
> Node::changed() method. 
> This solves bug 322735, where the project performance chart is not refreshed. 
> This is because there is a cache that does not get refreshed unless the 
> changed() method is called with the correct property.
> 
> * Instead of traversing all nodes looking for the one without parent, request 
> the project for that node using projectNode(). Just a small optimization. The 
> one node without parent is the project node, which can be easily requested, 
> instead of making a O(n2) loop.
> 
> 
> This addresses bug 322735.
>     http://bugs.kde.org/show_bug.cgi?id=322735
> 
> 
> Diffs
> -----
> 
>   plan/libs/kernel/kpttask.cpp da48be0 
>   plan/libs/models/kptnodechartmodel.cpp 655dfc8 
> 
> Diff: http://git.reviewboard.kde.org/r/111657/diff/
> 
> 
> Testing
> -------
> 
> Tested manually. The performance chart is refreshed correctly and there were 
> no apparent performance drops or other side-effects.
> 
> 
> Thanks,
> 
> Alvaro Soliverez
> 
>

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

Reply via email to