Review Request 110414: Implemented Save Action in Khipu

[Percy Camilo Triveño Aucahuasi]

> On May 27, 2013, 4:19 a.m., Percy Camilo Triveño Aucahuasi wrote:
> > Hi Punit, some comments:
> > 
> > I had to change json to JSON in cmake files in order to build (I'm in debian with qjson 0.8.1-1) ... and I don't know why, maybe some changes in qjson-devel or something (so -if it is possible- better we put a guard in cmake files for this)
> > 
> > Also, I really think that this kind of changes should be one of the last, let me explain: currently Khipu will crash when one want to create many plots and spaces, so there is not effective way to test if this changes are good enough; what I try to say is that first we need to have a consistent behaviour of Khipu in memory, so when the behaviour is validated and well tested then, later, we can to map from memory to disk. So once all (>90%) in Khipu goes well (at memory level) we'll comeback to this review.
> > 
> > Finally, -for future- I noted that all the code from save action is in the widget, I think we can try to decouple gui from application logic through classes like DataStore or Document (tip Document::save(const PlotsModel *, const SpacesModel *) maybe)
> > 
> > Thanks for your work, keep going :)
> > Percy
> >
> 
> Aleix Pol Gonzalez wrote:
>     Isn't open/save a good use case for creating and removing spaces? It can even be used as a test...

Yes, but only if:

1.- You just modify empty spaces (without plots) by the use of add/remove buttons; or
2.- You add spaces descriptions (text) manualy to the khipu file (this would be a not go)

The basic idea with spaces is this: each space can contain some plots, its a way to organize a plots. 

So, yes, currently it can be used as a test, but, without the feature of add/remove plots successfully to any space, then I'm sure it will not be a valid/complete test.


- Percy Camilo


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


On May 20, 2013, 3:45 p.m., Punit Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110414/
> -----------------------------------------------------------
> 
> (Updated May 20, 2013, 3:45 p.m.)
> 
> 
> Review request for KDE Edu, Aleix Pol Gonzalez and Percy Camilo Triveño Aucahuasi.
> 
> 
> Description
> -------
> 
> As we discussed the outline for khipu-specific file , I have implemented this feature. The attached diff contains the feature for save Action in Khipu. I have used qjson for file format.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt f7dc583 
>   src/CMakeLists.txt 0a50c54 
>   src/datastore.h a1276a7 
>   src/datastore.cpp 616e0cc 
>   src/mainwindow.h 1f059f8 
>   src/mainwindow.cpp 9d6e010 
>   src/plotseditor.h 47304c6 
>   src/plotseditor.cpp 106cdf5 
> 
> Diff: http://git.reviewboard.kde.org/r/110414/diff/
> 
> 
> Testing
> -------
> 
> Done..! worked perfect for any situtation I have come across. !
> 
> 
> Thanks,
> 
> Punit Mehta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20130528/b95c3fd2/attachment.html>