Discussion:
[Development] QButtonGroup: When the "right thing to do" is absolutely the wrong thing to do
b***@valdyas.org
2018-10-12 10:48:15 UTC
Permalink
So, there's this commit:

https://git.qt.io/consulting-usa/qtbase-xcb-rendering/commit/69335920f724d2d4a49924f373c4fef57c942831

"
Move QButtonGroup implementation from qabstractbutton.cpp to
qbuttongroup.cpp

Because it's the right thing to do.

Needed to introduce qbuttongroup_p.h because QAbstractButton
likes to poke around in QButtonGroup's private parts.

Fixed includes of qabstractbutton_p.h so it compiles on it's
own.

Change-Id: Ic7725277d2419754de273b2abd4790476edd0eb4
Reviewed-by: Olivier Goffart's avatarOlivier Goffart (Woboq GmbH)
<***@woboq.com>

"

Which of course breaks source compatibility. It's bad enough to have to
adapt one's codebase; but this also makes it impossible to bisect code
when Qt 5.11 is installed that had to be adapted.

I'm constantly running into this problem. Would it be too much to ask
that things like this just never ever happen again?


Boudewijn
Konstantin Ritt
2018-10-12 12:31:33 UTC
Permalink
It didn't.

Regards,
Konstantin


пт, 12 Пкт. 2018 г. в 13:59, Giuseppe D'Angelo via Development <
Hello,
Post by b***@valdyas.org
Which of course breaks source compatibility. It's bad enough to have to
adapt one's codebase; but this also makes it impossible to bisect code
when Qt 5.11 is installed that had to be adapted.
I'm constantly running into this problem. Would it be too much to ask
that things like this just never ever happen again?
In what way did that commit break source compatibility?
Thanks,
--
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development
Boudewijn Rempt
2018-10-13 07:03:08 UTC
Permalink
Post by Konstantin Ritt
It didn't.
Then what else made it necessary to suddenly add lots of #include
<QButtonGroup> lines?
--
https://www.krita.org
Olivier Goffart
2018-10-15 07:46:14 UTC
Permalink
Post by Boudewijn Rempt
Post by Konstantin Ritt
It didn't.
Then what else made it necessary to suddenly add lots of #include
<QButtonGroup> lines?
Maybe that was commit 000c76ada5cc21479fc479be16a7507fed6490f8 ?


The commit you mentioned does not touch any public headers and was already in
Qt 5.6. So unless you include private headers, in which case you are on your
own, there was no way it causes a source incompatible change. No wonder why the
replies you got were skeptical.
You could have stated directly what source incompatibilities you have seen.


Coming back to the actual problem of missing includes, this is indeed not the
first time that Qt broke source compatibility in that regards. (last time i
remember, it was requiring to include QDataStream, in Qt 5.5)

According to QUIP-6, 'Removing an include from a public header file' is an
acceptable source incompatible change. The rationale is that you should not
rely on indirect includes.
IMHO, this is a bit unfortunate, as this causes indeed source incompatible
change that makes it harder to upgrade Qt.
--
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
Mathias Hasselmann
2018-10-12 13:57:35 UTC
Permalink
Hi Boudewijn,

these kind of refactorings are neccessary to keep code maintainable I fear.

Anyway, it seems like you are maintaining a bunch of custom patches on
top of Qt? How about reducing your maintainance burden by upstreaming
them? Qt would benefit from bugfixes and new features. You would benefit
from reduced workload and responsiblity? This sounds like win-win to me,
do you agree?

Ciao,
Mathias
Post by b***@valdyas.org
https://git.qt.io/consulting-usa/qtbase-xcb-rendering/commit/69335920f724d2d4a49924f373c4fef57c942831
"
Move QButtonGroup implementation from qabstractbutton.cpp to
qbuttongroup.cpp
Because it's the right thing to do.
Needed to introduce qbuttongroup_p.h because QAbstractButton
likes to poke around in QButtonGroup's private parts.
Fixed includes of qabstractbutton_p.h so it compiles on it's
own.
Change-Id: Ic7725277d2419754de273b2abd4790476edd0eb4
Reviewed-by: Olivier Goffart's avatarOlivier Goffart (Woboq GmbH)
"
Which of course breaks source compatibility. It's bad enough to have
to adapt one's codebase; but this also makes it impossible to bisect
code when Qt 5.11 is installed that had to be adapted.
I'm constantly running into this problem. Would it be too much to ask
that things like this just never ever happen again?
Boudewijn
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development
Boudewijn Rempt
2018-10-13 07:02:02 UTC
Permalink
Post by Mathias Hasselmann
Hi Boudewijn,
these kind of refactorings are neccessary to keep code maintainable I fear.
Well... If code stops building from Qt 5 to another Qt 5, that means that
thousands of projects have to change their code. Thousands of projects have to
struggle. Is that really worth it? I always thought Qt came with a firm
promise not to break source compatibility except in new, major releases.
Post by Mathias Hasselmann
Anyway, it seems like you are maintaining a bunch of custom patches on
top of Qt? How about reducing your maintainance burden by upstreaming
them? Qt would benefit from bugfixes and new features. You would benefit
from reduced workload and responsiblity? This sounds like win-win to me,
do you agree?
Apart from one patch that disables wintab support because we've got our own,
those patches pretty much all come from Qt's bug tracker. We did try to
upstream the one big patch that made the opengl painter engine work again on
macOS, and that happened in the end.
--
https://www.krita.org
Loading...