If it compiles, all is fine.
Details
Diff Detail
- Repository
- R166 Spectacle
- Branch
- unused-include
- Lint
No Linters Available - Unit
No Unit Test Coverage
Thanks for the patch!
If it compiles, all is fine.
That's not enough ;) Changing the include order could also have side effects on the behaviour. Nevertheless, as far as I can see all is fine.
Clang and GCC (4.8 and 7) are happy, and basic functions seemed to work.
Please keep this.
Let me explain: config.h defines various constants which are injected by CMake. You are removing all occurrences but one, exploiting transitivity in the includes. However, that is very dangerous: As soon as this include chain breaks, e.g. if someone moves some includes around in an unrelated patch, the constants won't be defined anymore in that particular compilation unit, without any warning or error.
To prevent this mistake from happening, it's important to directly include config.h everywhere such constants are used.
src/Gui/ExportMenu.cpp | ||
---|---|---|
35 | Please keep this. | |
src/Gui/KSMainWindow.cpp | ||
21 | Please keep this. | |
src/Main.cpp | ||
35 | Please keep this. | |
src/PlatformBackends/KWinWaylandImageGrabber.cpp | ||
30 | This should be kept and changed to <cerrno>. | |
src/SpectacleCore.cpp | ||
38 | Please keep this. |
This supposedly breaks build https://bugs.kde.org/show_bug.cgi?id=391504 (built fine here, though, perhaps it only breaks if you do not have kipi or Purpose)