KScreenGenie moved to KDE Review

Alexander Potashev aspotashev at gmail.com
Sat Jul 4 12:59:14 BST 2015


Boudhayan,

Please find my comment below.

2015-07-04 14:29 GMT+03:00 Boudhayan Gupta <me at baloneygeek.com>:
>> You said "I can see exactly which components a single cpp file depends
>> on my looking at the headers". If you follow the recommendation in the
>> links [1,2], all the dependent components will be included in the .cpp
>> file, so you can see exactly what you want there. Any other issue with
>> it?
>
> No, some of them stay over in the header file. "Everything in one
> place", for a small application such as this, which is not a public
> library, takes precedence over minorly increased build times.

Again, if you follow the links [1,2], then Foo.cpp should have
#include <QString> even if Foo.h already contains it, in order to
account for possible removal of that #include from Foo.h in the
future. In this way, Foo.cpp acts like "one place for everything", the
header #including only a minimal set files and minimal forward
declarations to make it compile.

> That said, having to go through the commitdiffs for *four* unannounced
> commits, two of which were major, to find the *one* location where the
> real fix was and isolating that, is a huge waste of time which I could
> have otherwise used to make important pending fixes to the UI and
> program behaviour.

Sorry, I didn't expect the #include change to be questionable. Will
try to make my future commits more atomic.

-- 
Alexander Potashev




More information about the kde-core-devel mailing list