Review Request 122153: KD Chart

Stephen Leibowitz LibreStephen at gmail.com
Thu Jan 21 22:34:03 GMT 2016



> On Jan. 16, 2016, 12:31 a.m., Jarosław Staniek wrote:
> > @Friedrich What to do with this patch? Is it needed now or?
> 
> Friedrich W. H. Kossebau wrote:
>     Guess so. Still missing proper sets of files/data to test it, so can only blindly commit it, but perhaps better than to ignore. Will do in the next days.

I submitted the patch last year. I first submitted it to kdab.com, where it became issue KDCH-1020. The Status there is CLOSED, and the Resolution is FIXED. The Assignee was KDAB staffer Andreas Hartmetz. Here is his final comment, dated 14/Jan/15:

“I've changed the h/v line layout items to only grow in the lengthwise direction and removed the comments. The comments didn't add anything of value. I've also changed "< 360" to "<= 360". The latter is more consistent with line 211 and also with the previous if condition, which is inclusive at the edges of the value range (0...360). It should not change the behavior at all, due to reversedOrder = false by default. Well, actually the whole if and dependent code is redundant, it's really just documentation.”

Andreas said, “I've also changed "< 360" to "<= 360".”
I (Stephen) first suggested to kdab.com that "< 0" be changed to "< 360". I then slightly modified my suggestion to "<= 360". Here are the original and changed lines:
if ( ( angle >= 90 && angle < 180 ) || ( angle >= 270 && angle < 0 ) )
*change to*:
if ( ( angle >= 90 && angle < 180 ) || ( angle >= 270 && angle <= 360 ) )

Andreas mentioned line 211 in his comment. Here is the line:
} else if ( angle >= 270.0 && angle <= 360.0 ) {

Their line 211 corresponds to line 209 in Calligra’s repository at
https://projects.kde.org/projects/calligra/calligra/repository/entry/3rdparty/kdchart/src/KDChartStockDiagram_p.cpp?rev=calligra%2F2.9


- Stephen


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122153/#review91168
-----------------------------------------------------------


On Dec. 3, 2015, 7:58 a.m., Stephen Leibowitz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122153/
> -----------------------------------------------------------
> 
> (Updated Dec. 3, 2015, 7:58 a.m.)
> 
> 
> Review request for Calligra and Jarosław Staniek.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> These patches are also being made at kdab.com. Those who have a KDAB account can see the discussion in “Suggested Changes to KD Chart” at 
> https://quality.kdab.com/browse/KDCH-1020
> 
> Calligra’s kdchart and kdgantt files are out-of-date, even with the patches from the above paragraph. For example, “Compiler warnings” at
> http://mail.kde.org/pipermail/calligra-devel/2015-January/012762.html
> mentioned an error in KDChartPieDiagram.cpp. But the error is in a private function that was removed from the latest version (2.5.1) of KD Chart. KDAB will not patch previous versions. See “PieDiagram::drawPieSurface” at
> https://quality.kdab.com/browse/KDCH-1023
> 
> KDAB makes available source code for the latest and earlier versions of its KD Chart and other GPL licensed software at http://docs.kdab.com/
> 
> 
> Diffs
> -----
> 
>   3rdparty/kdchart/src/KDChartLayoutItems.cpp 095d2cd 
>   3rdparty/kdchart/src/KDChartStockDiagram_p.cpp d8636d7 
> 
> Diff: https://git.reviewboard.kde.org/r/122153/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Stephen Leibowitz
> 
>

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


More information about the calligra-devel mailing list