[Development] Are SiCs through #include cleanups considered acceptable?

[André Somers]

Simon Hausmann schreef op 9-4-2015 om 10:39:
> On Wednesday 8. April 2015 14.34.03 Olivier Goffart wrote:
>> On Wednesday 08 April 2015 13:13:19 Marc Mutz wrote:
>>> Hi,
>>>
>>> I have in the past fixed #include mistakes such as #include
>>> <qsharedpointer.h> for QSharedDataPointer, and even though each time the
>>> issue came up that this is a SiC change (but only for users that unduly
>>> rely on indirect includes), so far they were always accepted.
>>>
>>> When splitting off qHash() from qhash.h into qhashfunctions.h, I have come
>>> across many files that included qhash.h without using it, and likewise
>>> some
>>> for which qhashfunctions.h would suffice. One of them now got a -1 for
>>> being SiC.
>>>
>>> Can we please decide once and for all whether #include cleanups that are
>>> technically SiC are ok or not, if they only affect users that rely on
>>> indirect includes?
>>>
>>> My vote obviously goes to allowing them.
>> My vote goes against such gratuitous SiC changes.
>>
>> Each little SiC changes add up in frustration for the developer who upgrades
>> its Qt version.
>> So when it is easy to avoid, we should avoid it.
> I'm with Olivier on this one. Source compatibility is a key contribution to
> the success of Qt.
>
> For developers building their own software it may not be such a big deal, if
> they are experienced Qt developers. For somebody who just clones a random
> project off github and then tries to build it, such a "simple" source
> incompatible change becomes a deal breaker. I argue that there's a high
> probability that person is not familiar with these types of changes nor
> knowledgeable how to fix them.
>
I actually consider relying on implicitly included headers a bug, or at 
a same level as relying on private headers. The fact that Qt somehow 
implicitly includes header b defining class B when you inluded header a 
in order to use class A is an implementation detail IMHO. Nowhere does 
Qt promise stability of such implementation details, I think. Most often 
this is avoided in Qt via the use of PIMPL of course, but not always.

I think it *is* reasonable to remove such indirect includes if they are 
not needed for Qt itself. However, it is worth looking into how the 
thing was documented to begin with. In the case of qHash(), the 
documentation for that one actualy says the include is <QHash>. I think 
it is *not* reasonable to break that.

André