Review Request 110414: Implemented Save Action in Khipu

[Punit Mehta]

> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.h, line 99
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143494#file143494line99>
> >
> >     You don't initialize members here. = 0 goes to the constructor.
> >     
> >     Also do we really ned these attributes?

Yes.. we need that because I have implemented it in such a way that we need these variables to keep track of the  number spaces added and saved.!


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 325
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line325>
> >
> >     Unrelated to this patch. Also I'm unsure we want that.

Ok. I will remove this.


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 374
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line374>
> >
> >     You should _never_ use exit(0).
> >     
> >     I'm unsure what you're trying to do anyway.

I think I have used this to ensure that the application does not crash beacause of the null PlotItem . No problem , I will remove this also .!


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 422
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line422>
> >
> >     We have API for painting the contents of 2D and 3D graphs into an image. Maybe you should use that.

Aleix , which API you are talking about ? Is it Analitza or some class of Qt ? and is there any problem in the current approach ?


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/mainwindow.cpp, line 716
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143495#file143495line716>
> >
> >     Use QByteArray, not QString.
> >     
> >     Also fix indentation

OK. I will change this and send you the new diff as it gets ready .! 


> On May 13, 2013, 2:18 p.m., Aleix Pol Gonzalez wrote:
> > src/plotseditor.cpp, line 712
> > <http://git.reviewboard.kde.org/r/110414/diff/1/?file=143496#file143496line712>
> >
> >     This looks like terrible API. If you want to do this I'd suggest calling it addPlot and calling plotsModel() from within.

Aleix , I dont get this. You are telling to modify addPlot of analitzaplot ? 


- Punit


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


On May 13, 2013, 1:55 p.m., Punit Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110414/
> -----------------------------------------------------------
> 
> (Updated May 13, 2013, 1:55 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.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/20130514/0dce7988/attachment-0001.html>