-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120777/#review69092
-----------------------------------------------------------


I'm very confused about this whole patch. Can you explain what each of the 
models is supposed to be doing?

Also, why is this called Cloud? Finally, do you really want to call everything 
BalooSomething. It's using Baloo internally, but that's doesn't mean the users 
of these models need to explicitly be told that. They are more concerned with 
the features the model is providing, not the technology that it is using 
internally.


CMakeLists.txt
<https://git.reviewboard.kde.org/r/120777/#comment48309>

    You aren't really using FileMetaData



components/baloocloudmodel/balootimelinemodel.cpp
<https://git.reviewboard.kde.org/r/120777/#comment48306>

    One generally just adds such thing in Baloo if there are not there.
    
    I've added the operator != in Query



components/baloocloudmodel/balootimelinemodel.cpp
<https://git.reviewboard.kde.org/r/120777/#comment48307>

    This is incredibly inefficient.
    
    Could you please use Query::setDateFilter please.
    
    You should do the same for the month and days as well.


- Vishesh Handa


On Oct. 24, 2014, 12:21 p.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120777/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 12:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-mobile
> 
> 
> Description
> -------
> 
> At the moment, Baloo doesn't provide a timeline, which is something that we 
> need for the activefilebrowser.
> So this new component, is introducing support for the timeline.
> 
> Notes
> ===
> 
> * Baloocloud component contains the org.kde.baloo component inside it.The 
> reason behind that, is that the implementation for the timeline is kind of 
> terible because of its perfomance.
> * I have put the new component inside the plasma-mobile repository, for the 
> above reason. But if the Baloo team, wants it inside the baloo repo then i 
> can move it. I am fine with both approaches (keep it here or in the baloo 
> repository.
> * If someone has a better idea about the implementation, the pls shoot :)
>   
> 
> 
> Diffs
> -----
> 
>   components/baloocloudmodel/balootimelinemodel.h PRE-CREATION 
>   components/baloocloudmodel/balootimelinemodel.cpp PRE-CREATION 
>   components/baloocloudmodel/qmldir PRE-CREATION 
>   CMakeLists.txt 9466447 
>   components/CMakeLists.txt 536b60e 
>   components/baloocloudmodel/CMakeLists.txt PRE-CREATION 
>   components/baloocloudmodel/baloocloudmodel.h PRE-CREATION 
>   components/baloocloudmodel/baloocloudmodel.cpp PRE-CREATION 
>   components/baloocloudmodel/baloocloudplugin.h PRE-CREATION 
>   components/baloocloudmodel/baloocloudplugin.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120777/diff/
> 
> 
> Testing
> -------
> 
> Everything looks ok. The performance is not bad, except from the fact that 
> the implementation is a bit of hackish...
> 
> 
> Thanks,
> 
> Antonis Tsiapaliokas
> 
>

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

Reply via email to