Qt5.11 fix
ClosedPublic

Authored by garvitkhatri on May 31 2018, 7:11 AM.

Details

Reviewers
asemke

Diff Detail

Repository
R262 LabPlot
Lint
Lint Skipped
Unit
Unit Tests Skipped
garvitkhatri created this revision.May 31 2018, 7:11 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 31 2018, 7:11 AM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
garvitkhatri requested review of this revision.May 31 2018, 7:11 AM
asemke added inline comments.May 31 2018, 7:48 AM
src/commonfrontend/matrix/MatrixView.h
35 ↗(On Diff #35203)

This can go to the cpp-file, right?

src/kdefrontend/spreadsheet/FunctionValuesDialog.h
33

forward declaration should be enough here.

src/kdefrontend/widgets/FITSHeaderEditNewKeywordDialog.h
34

Also here the forward declaration and the include in the cpp-file should be enough.

garvitkhatri added inline comments.May 31 2018, 1:53 PM
src/commonfrontend/matrix/MatrixView.h
35 ↗(On Diff #35203)

Hey just to make sure I don't do this again in future, we add in .cpp when it is a requirement of implementation, and add this to .h file if this is the requirement of both implementation and definition?

asemke added inline comments.May 31 2018, 3:16 PM
src/commonfrontend/matrix/MatrixView.h
35 ↗(On Diff #35203)

We don't add include in the header if we don't need the full information about the class and it is sufficient to work with "incomplete types". This is the case if we only have pointers or references to objects declared in the header file you try to include. In such cases it is enough to have "forward declaration" in the header file and to include in the cpp file - this helps to reduce the dependencies between the source files and to speed up the compilation times. You can find more information on this if you search for "forward declaration", e.g. here http://en.cppreference.com/w/cpp/language/class

garvitkhatri added inline comments.Jun 1 2018, 9:48 AM
src/commonfrontend/matrix/MatrixView.h
35 ↗(On Diff #35203)

I have read this, will keep on investigating in future. I have made the relevant changes do let me know if they look good. I will push them to master once approved.

garvitkhatri marked an inline comment as done.Jun 2 2018, 5:41 AM
asemke accepted this revision.Jun 2 2018, 7:59 AM

Looks ok for me. I don't have Qt5.11 right now and cannot easily validate this. Please submit this change if it compiles with 5.11.

This revision is now accepted and ready to land.Jun 2 2018, 7:59 AM
garvitkhatri closed this revision.Jun 24 2018, 7:54 AM