Review Request 110414: Implemented Save Action in Khipu

[Percy Camilo Triveño Aucahuasi]

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


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


- Percy Camilo Triveño Aucahuasi


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/20130527/51e6285f/attachment.html>