Address T4491 making KReportDpi a private non-exported class
ClosedPublic

Authored by piggz on Dec 23 2016, 9:27 AM.

Details

Summary

Moved KReportDpi.h to KEReportDpi_p.h and associated changes

Test Plan

Ran autotests and kreportexample

Diff Detail

Repository
R14 KReport
Branch
t4491-kreportdpi
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz updated this revision to Diff 9315.Dec 23 2016, 9:27 AM
piggz retitled this revision from to Address T4491 making KReportDpi a private non-exported class.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptDec 23 2016, 9:27 AM
piggz updated this revision to Diff 9324.Dec 23 2016, 10:10 AM
  • Move include to cpp.
staniek requested changes to this revision.Dec 23 2016, 11:56 AM
staniek edited edge metadata.
staniek added inline comments.
src/wrtembed/KReportDpi.h
0

Is it used? If so it is somewhere else, we have no KoApplication.

This revision now requires changes to proceed.Dec 23 2016, 11:56 AM
piggz marked an inline comment as done.Dec 23 2016, 12:16 PM
piggz added inline comments.
src/wrtembed/KReportDpi.h
0

No, it is not used

piggz updated this revision to Diff 9329.Dec 23 2016, 12:17 PM
piggz edited edge metadata.
piggz marked an inline comment as done.
  • Remove unused legacy function

Hm hm hmm. After the removal I am thinking about this simplification:

Move the remains to KReportUtils_p.h (to KReportPrivate::dpi*()) and remove the KReportDpi*.* entirely.

staniek requested changes to this revision.Dec 23 2016, 4:00 PM
staniek edited edge metadata.
This revision now requires changes to proceed.Dec 23 2016, 4:00 PM
piggz updated this revision to Diff 9351.Dec 24 2016, 3:46 PM
piggz edited edge metadata.
  • Create a private static lib for KReportUtils_p
staniek requested changes to this revision.Dec 24 2016, 11:22 PM
staniek edited edge metadata.

Good job, one note

src/common/KReportUtils_p.h
23

This include is needed in KReportUtils_p.cpp not here?

This revision now requires changes to proceed.Dec 24 2016, 11:22 PM
piggz updated this revision to Diff 9353.Dec 25 2016, 3:42 PM
piggz edited edge metadata.
  • Move include to cpp and remove unnescessary cmake configs
staniek accepted this revision.Dec 27 2016, 11:40 AM
staniek edited edge metadata.

Nice!

This revision is now accepted and ready to land.Dec 27 2016, 11:40 AM
This revision was automatically updated to reflect the committed changes.
staniek added inline comments.Dec 30 2016, 6:33 PM
src/common/KReportUtils_p.cpp
2

undo:

  • copyright removal :)
  • typo in email