<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122867/">https://git.reviewboard.kde.org/r/122867/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 10th, 2015, 4:35 a.m. UTC, <b>Thorsten Zachmann</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch looks good to me.</p></pre>
</blockquote>
<p>On April 5th, 2015, 1:23 a.m. UTC, <b>Robert Mathias Marmorstein</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I downloaded the tests from calligratests, but haven't been able to get the QPro plugin to work well enough to test this at all. I'm really hesitant to commit without further testing. To be completely frank, this plugin seems to be in pretty bad shape. Someone with more experience than me should probably look it over and decide if it's worth keeping.</p></pre>
</blockquote>
<p>On April 5th, 2015, 9:40 a.m. UTC, <b>Camilla Boemann</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't think anyone around knows this filter better than you would, and if you cant find any sampe qpro files on the web then we will just have to hope for the best i guess</p></pre>
</blockquote>
<p>On April 5th, 2015, noon UTC, <b>Boudewijn Rempt</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If the filter is essentially untestable because there are no more files out in the wild to test with, then I'd agree that removing it is the best option.</p></pre>
</blockquote>
<p>On April 5th, 2015, 6:01 p.m. UTC, <b>Robert Mathias Marmorstein</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The problem isn't really finding test cases -- the calligratests svn repo has some -- it's that the "stream" part of the plugin that parses that file doesn't seem to work properly (even before these patches) so it's hard to test.</p></pre>
</blockquote>
<p>On April 16th, 2015, 7:01 a.m. UTC, <b>Camilla Boemann</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think you were careful - I know you have coded for a long time - just go ahead and push and close this review</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Okay! Will do.</p></pre>
<br />
<p>- Robert Mathias</p>
<br />
<p>On April 16th, 2015, 2:44 p.m. UTC, Robert Mathias Marmorstein wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Calligra.</div>
<div>By Robert Mathias Marmorstein.</div>
<p style="color: grey;"><i>Updated April 16, 2015, 2:44 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The strstream class has been deprecated for some time in favor of the sstream classes. This patch replaces strstream with sstreams.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">After the change calligra compiles and passes the same unit tests as before. Unfortunately, I don't have any qpro files lying around to test this on.</p></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>filters/sheets/qpro/libqpro/src/formula.cc <span style="color: grey">(c5d6021)</span></li>
<li>filters/sheets/qpro/libqpro/src/record.cc <span style="color: grey">(bde5e7d)</span></li>
<li>filters/sheets/qpro/libqpro/src/stream.cc <span style="color: grey">(4126aee)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122867/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>