Review Request 107866: Pivot Tables for Calligra Sheets

[Marijn Kruisselbrink]

> On Jan. 14, 2013, 11:17 p.m., Inge Wallin wrote:
> > I think it's very high time that somebody who knows Sheets took a look at this review request. Nobody should have to wait 3 weeks to get a review, especially for something as important as this.
> 
> Marijn Kruisselbrink wrote:
>     Yes, I'm sorry; I took a quick look earlier, and I don't think most of the issues are actually related to Sheets. It is definitely an important feature to have, but in it's current state (with buttons that don't do anything, UI that is confusing, limitations that are not visible in the UI), I don't think it is ready to be part of a release. It would have been nice if this could somehow have been a plugin that we could ship separately, but I don't think that is quite possible at the moment.
>     Most of these things are of course to blame on me, for not having been more active during the development of this branch/summer of code project. I'll try to spend some time during my commutes this week to come up with some way this can at least be merged without actually confusing users with it (or maybe thinking of a way the functionality can be moved to be a plugin), as well as doing a more detailed review of the implementation itself.
>     
>     Additionally from a quick look, there are some issues with coding style/conventions (files should have CamelCase names), qDebug()s that should probably be removed.
> 
> Marijn Kruisselbrink wrote:
>     (also it's hard to comment on the actual code, since there doesn't seem to be an actual diff uploaded to this review request)
> 
> Jigar Raisinghani wrote:
>     I have added a diff for new files added(Revision 1) & some other files which were edited(Revision 2). I will take care of the CamelCase, qDebug.
>     
>     Moreover, I had deliberately left some of the buttons in the UI non-functional(as they are yet to be developed). I can remove the non-functional buttons and tweak the UI to make it possible for at least a certain release. If not a "Ship It!", I just wanted a feedback so that I could move ahead. The functionality for non-functional buttons could be added later and so could the buttons be. For the time being I could remove those.

Okay, I pushed a change to your branch that moves all your changes to plugins/staging/pivottables (and turned it into a plugin). With that done, I don't think there is much stopping this from being merged into master and developing it there further.


- Marijn


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


On Jan. 15, 2013, 7:29 p.m., Jigar Raisinghani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107866/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2013, 7:29 p.m.)
> 
> 
> Review request for Calligra and Marijn Kruisselbrink.
> 
> 
> Description
> -------
> 
> I had built Pivot Tables as part of my GSoC 2012 project. Some minor features were remaining which i finished lately. I have committed my latest code to the branch. 
> 
> How to use:   Blog link : http://jigarraisinghani.blogspot.in/2012/07/pivot-tablesupdate-here-is-update-about.html
>               Video link: http://www.youtube.com/watch?v=uz2PGVNyseA
> 
>  
> 
> Note:
> 1) Please drop only the fields containing non alphabets in "Values" and support is built for only 1 field in "Values". You can drop various in "Rows" & "Columns".
> 
> Features Still to be built:
> 1) Page Fields: Only GUI is there, but the functionality is yet to be added.
> 2) Pivot Options: The functionality only contains simple functions yet. The Base Field, Base Item functionalities are yet to be added.
> 
> Also note that pivot tables only makes sense if used in correct manner.
> 
> 
> Diffs
> -----
> 
>   sheets/CMakeLists.txt f617322 
>   sheets/sheets.rc 7eae858 
> 
> Diff: http://git.reviewboard.kde.org/r/107866/diff/
> 
> 
> Testing
> -------
> 
> I have tested the code for different test cases(data sets) and it works fine.
> 
> 
> Thanks,
> 
> Jigar Raisinghani
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130116/f8ccd183/attachment.htm>