The patch fixes tons of -Wsuggest-override warnings. Fix based on the results of clang-tidy run with modernize-use-override enabled check.
Diff Detail
Diff Detail
- Repository
- R478 KDiagram: Libraries for diagrams
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
I don't see any problems with this, but I have not tested and I'm not a compiler wizard...
Frederic must have the last word, I think.
Comment Actions
Would be an improvement, thanks for the work, @antonanikin. Patch not tested myself though, assuming you have done well enough :)
@mwolff @ahartmetz Can you tell how much this patch might complicate any potential further patch imports from KDChart? That would be the only blocker here.
Comment Actions
Patch not tested myself though, assuming you have done well enough :)
Hi, Friedrich. My testing was in building/using heaptrack checker which uses kdiagrams library - no problems detected.
Also, all built-in tests passed:
$ make test Running tests... Test project /home/htower/develop/kde/BUILD/kdiagram Start 1: appstreamtest 1/20 Test #1: appstreamtest ......................... Passed 0.02 sec Start 2: TestKChartAttributesModel 2/20 Test #2: TestKChartAttributesModel ............. Passed 0.05 sec Start 3: TestKChartAxisOwnership 3/20 Test #3: TestKChartAxisOwnership ............... Passed 0.10 sec Start 4: TestKChartBarDiagrams 4/20 Test #4: TestKChartBarDiagrams ................. Passed 0.05 sec Start 5: CartesianDiagramDataCompressorTests 5/20 Test #5: CartesianDiagramDataCompressorTests ... Passed 0.05 sec Start 6: TestCartesianPlanes 6/20 Test #6: TestCartesianPlanes ................... Passed 0.06 sec Start 7: TestChartElementOwnership 7/20 Test #7: TestChartElementOwnership ............. Passed 0.14 sec Start 8: TestCloning 8/20 Test #8: TestCloning ........................... Passed 0.10 sec Start 9: TestDrawIntoPainter 9/20 Test #9: TestDrawIntoPainter ................... Passed 22.48 sec Start 10: TestLegends 10/20 Test #10: TestLegends ........................... Passed 0.09 sec Start 11: TestLineDiagrams 11/20 Test #11: TestLineDiagrams ...................... Passed 0.05 sec Start 12: TestMeasure 12/20 Test #12: TestMeasure ........................... Passed 0.06 sec Start 13: TestKChartPalette 13/20 Test #13: TestKChartPalette ..................... Passed 0.05 sec Start 14: TestParamVsParam 14/20 Test #14: TestParamVsParam ...................... Passed 0.11 sec Start 15: TestPieDiagrams 15/20 Test #15: TestPieDiagrams ....................... Passed 0.07 sec Start 16: TestPolarDiagrams 16/20 Test #16: TestPolarDiagrams ..................... Passed 0.06 sec Start 17: TestPolarPlanes 17/20 Test #17: TestPolarPlanes ....................... Passed 0.05 sec Start 18: TestQLayout 18/20 Test #18: TestQLayout ........................... Passed 0.11 sec Start 19: TestRelativePosition 19/20 Test #19: TestRelativePosition .................. Passed 0.07 sec Start 20: TestWidgetElementOwnership 20/20 Test #20: TestWidgetElementOwnership ............ Passed 0.14 sec 100% tests passed, 0 tests failed out of 20 Total Test time (real) = 23.91 sec
Comment Actions
Hi @antonanikin , so while waiting for kdabians to tell how much this patch might complicate any further merges from kdchart, lmontel has simply created facts and without discussion committed a similar patch directly (though using Q_DECL_OVERRIDE) on his current mission across all KDE repos. Oh well.
So thanks again for your work, sorry for not having been more pushy myself to get the answers I was looking for.
At least your one goal to have the kdiagram API modernized is achieved, let's put it like this :)
So please close this revision now (or update if there is something left-over from lmontel's commit).
Comment Actions
So please close this revision now (or update if there is something left-over from lmontel's commit).
Ok :)