<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107866/">http://git.reviewboard.kde.org/r/107866/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 14th, 2013, 11:17 p.m. UTC, <b>Inge Wallin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On January 15th, 2013, 12:19 a.m. UTC, <b>Marijn Kruisselbrink</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On January 15th, 2013, 12:25 a.m. UTC, <b>Marijn Kruisselbrink</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">(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)</pre>
</blockquote>
<p>On January 15th, 2013, 7:41 p.m. UTC, <b>Jigar Raisinghani</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- Marijn</p>
<br />
<p>On January 15th, 2013, 7:29 p.m. UTC, Jigar Raisinghani wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra and Marijn Kruisselbrink.</div>
<div>By Jigar Raisinghani.</div>
<p style="color: grey;"><i>Updated Jan. 15, 2013, 7:29 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have tested the code for different test cases(data sets) and it works fine.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>sheets/CMakeLists.txt <span style="color: grey">(f617322)</span></li>
<li>sheets/sheets.rc <span style="color: grey">(7eae858)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107866/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>